[geary/wip/181-special-folder-dupes: 3/3] Revamp Geary.FolderPath implementation



commit 1949fcd369e45f5aaa229cfefddd583a241dba9e
Author: Michael Gratton <mike vee net>
Date:   Mon Jan 14 17:26:18 2019 +1100

    Revamp Geary.FolderPath implementation
    
    Convert getters that look like properties into actual properties,
    remove unused and redundant public and internal API, convert
    implementation to use a tree that aucyally maintains references between
    steps, rather than each creating a new list of path steps and
    manipulating that.

 src/client/accounts/accounts-manager.vala          |  46 ++-
 .../folder-list/folder-list-account-branch.vala    |   4 +-
 src/engine/api/geary-folder-path.vala              | 404 ++++++++++-----------
 src/engine/api/geary-folder.vala                   |   6 +-
 src/engine/imap-db/imap-db-account.vala            |  14 +-
 .../imap-engine/imap-engine-generic-account.vala   |   6 +-
 .../imap/message/imap-mailbox-specifier.vala       |  25 +-
 src/engine/imap/transport/imap-client-session.vala |  24 +-
 test/engine/api/geary-folder-path-test.vala        |  67 +++-
 test/engine/imap-db/imap-db-account-test.vala      |  10 +-
 10 files changed, 318 insertions(+), 288 deletions(-)
---
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index 96167748..426cb28a 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -1152,7 +1152,9 @@ public class Accounts.AccountConfigV1 : AccountConfig, GLib.Object {
                              string key,
                              Geary.FolderPath? path) {
         if (path != null) {
-            config.set_string_list(key, path.as_list());
+            config.set_string_list(
+                key, new Gee.ArrayList<string>.wrap(path.as_array())
+            );
         }
     }
 
@@ -1311,17 +1313,37 @@ public class Accounts.AccountConfigLegacy : AccountConfig, GLib.Object {
             );
         }
 
-        Gee.LinkedList<string> empty = new Gee.LinkedList<string>();
-        config.set_string_list(DRAFTS_FOLDER_KEY, (info.drafts_folder_path != null
-            ? info.drafts_folder_path.as_list() : empty));
-        config.set_string_list(SENT_MAIL_FOLDER_KEY, (info.sent_folder_path != null
-            ? info.sent_folder_path.as_list() : empty));
-        config.set_string_list(SPAM_FOLDER_KEY, (info.spam_folder_path != null
-            ? info.spam_folder_path.as_list() : empty));
-        config.set_string_list(TRASH_FOLDER_KEY, (info.trash_folder_path != null
-            ? info.trash_folder_path.as_list() : empty));
-        config.set_string_list(ARCHIVE_FOLDER_KEY, (info.archive_folder_path != null
-            ? info.archive_folder_path.as_list() : empty));
+        Gee.ArrayList<string> empty = new Gee.ArrayList<string>();
+        config.set_string_list(
+            DRAFTS_FOLDER_KEY,
+            (info.drafts_folder_path != null
+             ? new Gee.ArrayList<string>.wrap(info.drafts_folder_path.as_array())
+             : empty)
+        );
+        config.set_string_list(
+            SENT_MAIL_FOLDER_KEY,
+            (info.sent_folder_path != null
+             ? new Gee.ArrayList<string>.wrap(info.sent_folder_path.as_array())
+             : empty)
+        );
+        config.set_string_list(
+            SPAM_FOLDER_KEY,
+            (info.spam_folder_path != null
+             ? new Gee.ArrayList<string>.wrap(info.spam_folder_path.as_array())
+             : empty)
+        );
+        config.set_string_list(
+            TRASH_FOLDER_KEY,
+            (info.trash_folder_path != null
+             ? new Gee.ArrayList<string>.wrap(info.trash_folder_path.as_array())
+             : empty)
+        );
+        config.set_string_list(
+            ARCHIVE_FOLDER_KEY,
+            (info.archive_folder_path != null
+             ? new Gee.ArrayList<string>.wrap(info.archive_folder_path.as_array())
+             : empty)
+        );
 
         config.set_bool(SAVE_DRAFTS_KEY, info.save_drafts);
     }
diff --git a/src/client/folder-list/folder-list-account-branch.vala 
b/src/client/folder-list/folder-list-account-branch.vala
index 6a87da02..70587d9e 100644
--- a/src/client/folder-list/folder-list-account-branch.vala
+++ b/src/client/folder-list/folder-list-account-branch.vala
@@ -98,11 +98,11 @@ public class FolderList.AccountBranch : Sidebar.Branch {
                 graft(get_root(), user_folder_group);
             }
         } else {
-            Sidebar.Entry? entry = folder_entries.get(folder.path.get_parent());
+            Sidebar.Entry? entry = folder_entries.get(folder.path.parent);
             if (entry != null)
                 graft_point = entry;
         }
-        
+
         // Due to how we enumerate folders on the server, it's unfortunately
         // possible now to have two folders that we'd put in the same place in
         // our tree.  In that case, we just ignore the second folder for now.
diff --git a/src/engine/api/geary-folder-path.vala b/src/engine/api/geary-folder-path.vala
index 4d398d99..4f01d168 100644
--- a/src/engine/api/geary-folder-path.vala
+++ b/src/engine/api/geary-folder-path.vala
@@ -13,14 +13,28 @@
  * @see FolderRoot
  */
 
-public class Geary.FolderPath : BaseObject, Gee.Hashable<Geary.FolderPath>,
-    Gee.Comparable<Geary.FolderPath> {
+public class Geary.FolderPath :
+    BaseObject, Gee.Hashable<FolderPath>, Gee.Comparable<FolderPath> {
 
 
-    /**
-     * The name of this folder (without any child or parent names or delimiters).
-     */
-    public string basename { get; private set; }
+    // Workaround for Vala issue #659. See children below.
+    private class FolderPathWeakRef {
+
+        GLib.WeakRef weak_ref;
+
+        public FolderPathWeakRef(FolderPath path) {
+            this.weak_ref = GLib.WeakRef(path);
+        }
+
+        public FolderPath? get() {
+            return this.weak_ref.get() as FolderPath;
+        }
+
+    }
+
+
+    /** The base name of this folder, excluding parents. */
+    public string name { get; private set; }
 
     /**
      * Whether this path is lexiographically case-sensitive.
@@ -29,111 +43,68 @@ public class Geary.FolderPath : BaseObject, Gee.Hashable<Geary.FolderPath>,
      */
     public bool case_sensitive { get; private set; }
 
-    /**
-     * Determines if this path is a root folder path.
-     */
-    public virtual bool is_root {
-        get { return this.path == null || this.path.size == 0; }
+    /** Determines if this path is a root folder path. */
+    public bool is_root {
+        get { return this.parent == null; }
     }
 
-    /**
-     * Determines if this path is a child of the root folder.
-     */
+    /** Determines if this path is a child of the root folder. */
     public bool is_top_level {
         get {
-            FolderPath? parent = get_parent();
+            FolderPath? parent = parent;
             return parent != null && parent.is_root;
         }
     }
 
+    /** Returns the parent of this path. */
+    public FolderPath? parent { get; private set; }
+
+    private string[] path;
 
-    private Gee.List<Geary.FolderPath>? path = null;
-    private uint stored_hash = uint.MAX;
+    // Would use a `weak FolderPath` value type for this map instead of
+    // the custom class, but we can't currently reassign built-in
+    // weak refs back to a strong ref at the moment, nor use a
+    // GLib.WeakRef as a generics param. See Vala issue #659.
+    private Gee.Map<string,FolderPathWeakRef?> children =
+        new Gee.HashMap<string,FolderPathWeakRef?>();
+
+    private uint? stored_hash = null;
 
 
     /** Constructor only for use by {@link FolderRoot}. */
     internal FolderPath() {
-        this.basename = "";
+        this.name = "";
+        this.parent = null;
         this.case_sensitive = false;
+        this.path = new string[0];
     }
 
-    private FolderPath.child(Gee.List<Geary.FolderPath> path, string basename, bool case_sensitive) {
-        assert(path[0] is FolderRoot);
-        
-        this.path = path;
-        this.basename = basename;
+    private FolderPath.child(FolderPath parent,
+                             string name,
+                             bool case_sensitive) {
+        this.parent = parent;
+        this.name = name;
         this.case_sensitive = case_sensitive;
+        this.path = parent.path.copy();
+        this.path += name;
     }
 
     /**
      * Returns the {@link FolderRoot} of this path.
      */
     public Geary.FolderRoot get_root() {
-        return (FolderRoot) ((path != null && path.size > 0) ? path[0] : this);
-    }
-    
-    /**
-     * Returns the parent {@link FolderPath} of this folder or null if this is the root.
-     *
-     * @see is_root
-     */
-    public Geary.FolderPath? get_parent() {
-        return (path != null && path.size > 0) ? path.last() : null;
-    }
-    
-    /**
-     * Returns the number of folders in this path, not including any children of this object.
-     */
-    public int get_path_length() {
-        // include self, which is not stored in the path list
-        return (path != null) ? path.size + 1 : 1;
-    }
-    
-    /**
-     * Returns the {@link FolderPath} object at the index, with this FolderPath object being
-     * the farthest child.
-     *
-     * Root is at index 0 (zero).
-     *
-     * Returns null if index is out of bounds.  There is always at least one element in the path,
-     * namely this one, meaning zero is always acceptable and that index[length - 1] will always
-     * return this object.
-     *
-     * @see get_path_length
-     */
-    public Geary.FolderPath? get_folder_at(int index) {
-        // include self, which is not stored in the path list ... essentially, this logic makes it
-        // look like "this" is stored at the end of the path list
-        if (path == null)
-            return (index == 0) ? this : null;
-        
-        int length = path.size;
-        if (index < length)
-            return path[index];
-        
-        if (index == length)
-            return this;
-        
-        return null;
+        FolderPath? path = this;
+        while (path.parent != null) {
+            path = path.parent;
+        }
+        return (FolderRoot) path;
     }
-    
+
     /**
-     * Returns the {@link FolderPath} as a List of {@link basename} strings, this FolderPath's
-     * being the last in the list.
-     *
-     * Thus, the list should have at least one element.
+     * Returns an array of the names of non-root elements in the path.
      */
-    public Gee.List<string> as_list() {
-        Gee.List<string> list = new Gee.ArrayList<string>();
-        
-        if (path != null) {
-            foreach (Geary.FolderPath folder in path)
-                list.add(folder.basename);
-        }
-        
-        list.add(basename);
-        
-        return list;
+    public string[] as_array() {
+        return this.path;
     }
 
     /**
@@ -144,39 +115,25 @@ public class Geary.FolderPath : BaseObject, Gee.Hashable<Geary.FolderPath>,
      * {@link Trillian.UNKNOWN}, then {@link
      * FolderRoot.default_case_sensitivity} is used.
      */
-    public virtual Geary.FolderPath
-        get_child(string basename,
+    public virtual FolderPath
+        get_child(string name,
                   Trillian is_case_sensitive = Trillian.UNKNOWN) {
-        // Build the child's path, which is this node's path plus this node
-        Gee.List<FolderPath> child_path = new Gee.ArrayList<FolderPath>();
-        if (path != null)
-            child_path.add_all(path);
-        child_path.add(this);
-
-        return new FolderPath.child(
-            child_path,
-            basename,
-            is_case_sensitive.to_boolean(get_root().default_case_sensitivity)
-        );
-    }
-
-    /**
-     * Returns true if the other {@link FolderPath} has the same parent as this one.
-     *
-     * Like {@link equal_to} and {@link compare_to}, this comparison the comparison is
-     * lexiographic, not by reference.
-     */
-    public bool has_same_parent(FolderPath other) {
-        FolderPath? parent = get_parent();
-        FolderPath? other_parent = other.get_parent();
-        
-        if (parent == other_parent)
-            return true;
-        
-        if (parent != null && other_parent != null)
-            return parent.equal_to(other_parent);
-        
-        return false;
+        FolderPath? child = null;
+        FolderPathWeakRef? child_ref = this.children.get(name);
+        if (child_ref != null) {
+            child = child_ref.get();
+        }
+        if (child == null) {
+            child = new FolderPath.child(
+                this,
+                name,
+                is_case_sensitive.to_boolean(
+                    get_root().default_case_sensitivity
+                )
+            );
+            this.children.set(name, new FolderPathWeakRef(child));
+        }
+        return child;
     }
 
     /**
@@ -184,124 +141,96 @@ public class Geary.FolderPath : BaseObject, Gee.Hashable<Geary.FolderPath>,
      */
     public bool is_descendant(FolderPath target) {
         bool is_descendent = false;
-        Geary.FolderPath? path = target.get_parent();
+        FolderPath? path = target.parent;
         while (path != null) {
             if (path.equal_to(this)) {
                 is_descendent = true;
                 break;
             }
-            path = path.get_parent();
+            path = path.parent;
         }
         return is_descendent;
     }
 
-    private uint get_basename_hash() {
-        return case_sensitive ? str_hash(basename) : str_hash(basename.down());
-    }
-    
-    private int compare_internal(Geary.FolderPath other, bool allow_case_sensitive, bool normalize) {
-        if (this == other)
-            return 0;
-        
-        // walk elements using as_list() as that includes the basename (whereas path does not),
-        // avoids the null problem, and makes comparisons straightforward
-        Gee.List<string> this_list = as_list();
-        Gee.List<string> other_list = other.as_list();
-        
-        // if paths exist, do comparison of each parent in order
-        int min = int.min(this_list.size, other_list.size);
-        for (int ctr = 0; ctr < min; ctr++) {
-            string this_element = this_list[ctr];
-            string other_element = other_list[ctr];
-            
-            if (normalize) {
-                this_element = this_element.normalize();
-                other_element = other_element.normalize();
-            }
-            if (!allow_case_sensitive
-                // if either case-sensitive, then comparison is CS
-                || (!get_folder_at(ctr).case_sensitive && !other.get_folder_at(ctr).case_sensitive)) {
-                this_element = this_element.casefold();
-                other_element = other_element.casefold();
-            }
-            
-            int result = this_element.collate(other_element);
-            if (result != 0)
-                return result;
-        }
-        
-        // paths up to the min element count are equal, shortest path is less-than, otherwise
-        // equal paths
-        return this_list.size - other_list.size;
-    }
-    
     /**
-     * Does a Unicode-normalized, case insensitive match.  Useful for getting a rough idea if
-     * a folder matches a name, but shouldn't be used to determine strict equality.
+     * Does a Unicode-normalized, case insensitive match.  Useful for
+     * getting a rough idea if a folder matches a name, but shouldn't
+     * be used to determine strict equality.
      */
-    public int compare_normalized_ci(Geary.FolderPath other) {
+    public int compare_normalized_ci(FolderPath other) {
         return compare_internal(other, false, true);
     }
-    
+
     /**
      * {@inheritDoc}
      *
-     * Comparisons for Geary.FolderPath is defined as (a) empty paths are less-than non-empty paths
-     * and (b) each element is compared to the corresponding path element of the other FolderPath
-     * following collation rules for casefolded (case-insensitive) compared, and (c) shorter paths
-     * are less-than longer paths, assuming the path elements are equal up to the shorter path's
+     * Comparisons for FolderPath is defined as (a) empty paths
+     * are less-than non-empty paths and (b) each element is compared
+     * to the corresponding path element of the other FolderPath
+     * following collation rules for casefolded (case-insensitive)
+     * compared, and (c) shorter paths are less-than longer paths,
+     * assuming the path elements are equal up to the shorter path's
      * length.
      *
      * Note that {@link FolderPath.case_sensitive} affects comparisons.
      *
-     * Returns -1 if this path is lexiographically before the other, 1 if its after, and 0 if they
-     * are equal.
+     * Returns -1 if this path is lexiographically before the other, 1
+     * if its after, and 0 if they are equal.
      */
-    public int compare_to(Geary.FolderPath other) {
+    public int compare_to(FolderPath other) {
         return compare_internal(other, true, false);
     }
-    
+
     /**
      * {@inheritDoc}
      *
      * Note that {@link FolderPath.case_sensitive} affects comparisons.
      */
     public uint hash() {
-        if (stored_hash != uint.MAX)
-            return stored_hash;
-        
-        // always one element in path
-        stored_hash = get_folder_at(0).get_basename_hash();
-        
-        int path_length = get_path_length();
-        for (int ctr = 1; ctr < path_length; ctr++)
-            stored_hash ^= get_folder_at(ctr).get_basename_hash();
-        
-        return stored_hash;
-    }
-    
-    private bool is_basename_equal(string cmp, bool other_cs) {
-        // case-sensitive comparison if either is sensitive
-        return (other_cs || case_sensitive) ? (basename == cmp) : (basename.down() == cmp.down());
+        if (this.stored_hash == null) {
+            this.stored_hash = 0;
+            FolderPath? path = this;
+            while (path != null) {
+                this.stored_hash ^= (case_sensitive)
+                    ? str_hash(path.name) : str_hash(path.name.down());
+                path = path.parent;
+            }
+        }
+        return this.stored_hash;
     }
-    
-    /**
-     * {@inheritDoc}
-     */
-    public bool equal_to(Geary.FolderPath other) {
-        int path_length = get_path_length();
-        if (other.get_path_length() != path_length)
-            return false;
-        
-        for (int ctr = 0; ctr < path_length; ctr++) {
-            // this should never return null as length is already checked
-            FolderPath? other_folder = other.get_folder_at(ctr);
-            assert(other_folder != null);
-            
-            if (!get_folder_at(ctr).is_basename_equal(other_folder.basename, other_folder.case_sensitive))
+
+    /** {@inheritDoc} */
+    public bool equal_to(FolderPath other) {
+        if (this == other) {
+            return true;
+        }
+
+        FolderPath? a = this;
+        FolderPath? b = other;
+        while (a != null && b != null) {
+            if (a == b) {
+                return true;
+            }
+
+            if ((a != null && b == null) ||
+                (a == null && b != null)) {
                 return false;
+            }
+
+            if (a.case_sensitive || b.case_sensitive) {
+                if (a.name != b.name) {
+                    return false;
+                }
+            } else {
+                if (a.name.down() != b.name.down()) {
+                    return false;
+                }
+            }
+
+            a = a.parent;
+            b = b.parent;
         }
-        
+
         return true;
     }
 
@@ -314,16 +243,68 @@ public class Geary.FolderPath : BaseObject, Gee.Hashable<Geary.FolderPath>,
      * instead. This method is useful for debugging and logging only.
      */
     public string to_string() {
+        const char SEP = '>';
         StringBuilder builder = new StringBuilder();
-        if (this.path != null) {
-            foreach (Geary.FolderPath folder in this.path) {
-                builder.append(folder.basename);
-                builder.append_c('>');
+        if (this.is_root) {
+            builder.append_c(SEP);
+        } else {
+            foreach (string name in this.path) {
+                builder.append_c(SEP);
+                builder.append(name);
             }
         }
-        builder.append(basename);
         return builder.str;
     }
+
+    private int compare_internal(FolderPath other,
+                                 bool allow_case_sensitive,
+                                 bool normalize) {
+        if (this == other)
+            return 0;
+
+        FolderPath a = this;
+        FolderPath b = other;
+
+        // Get the common-length prefix of both
+        while (a.path.length != b.path.length) {
+            if (a.path.length > b.path.length) {
+                a = a.parent;
+            } else if (b.path.length > a.path.length) {
+                b = b.parent;
+            }
+        }
+
+        // Compare the common-length prefixes of both
+        while (a != null && b != null) {
+            string a_name = a.name;
+            string b_name = b.name;
+
+            if (normalize) {
+                a_name = a_name.normalize();
+                b_name = b_name.normalize();
+            }
+
+            if (!allow_case_sensitive
+                // if either case-sensitive, then comparison is CS
+                || (!a.case_sensitive && !b.case_sensitive)) {
+                a_name = a_name.casefold();
+                b_name = b_name.casefold();
+            }
+
+            int result = a_name.collate(b_name);
+            if (result != 0) {
+                return result;
+            }
+
+            a = a.parent;
+            b = b.parent;
+        }
+
+        // paths up to the min element count are equal, shortest path
+        // is less-than, otherwise equal paths
+        return this.path.length - other.path.length;
+    }
+
 }
 
 /**
@@ -334,14 +315,9 @@ public class Geary.FolderPath : BaseObject, Gee.Hashable<Geary.FolderPath>,
  * Because all FolderPaths hold references to their parents, this
  * element can be retrieved with {@link FolderPath.get_root}.
  */
-public class Geary.FolderRoot : Geary.FolderPath {
+public class Geary.FolderRoot : FolderPath {
 
 
-    /** {@inheritDoc} */
-    public override bool is_root {
-        get { return true; }
-    }
-
     /**
      * The default case sensitivity of descendant folders.
      *
diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala
index afdd1ee7..227379f7 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -451,16 +451,16 @@ public abstract class Geary.Folder : BaseObject {
     protected virtual void notify_display_name_changed() {
         display_name_changed();
     }
-    
+
     /**
      * Returns a name suitable for displaying to the user.
      *
-     * Default is to display the basename of the Folder's path, unless it's a special folder,
+     * Default is to display the name of the Folder's path, unless it's a special folder,
      * in which case {@link SpecialFolderType.get_display_name} is returned.
      */
     public virtual string get_display_name() {
         return (special_folder_type == Geary.SpecialFolderType.NONE)
-            ? path.basename : special_folder_type.get_display_name();
+            ? path.name : special_folder_type.get_display_name();
     }
 
     /** Determines if a folder has been opened, and if so in which way. */
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index 2b67edd7..d221af52 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -391,7 +391,7 @@ private class Geary.ImapDB.Account : BaseObject {
         }
 
         if (Imap.MailboxSpecifier.folder_path_is_inbox(path) &&
-            !Imap.MailboxSpecifier.is_canonical_inbox_name(path.basename)) {
+            !Imap.MailboxSpecifier.is_canonical_inbox_name(path.name)) {
             // Don't add faux inboxes
             throw new ImapError.NOT_SUPPORTED(
                 "Inbox has : %s", path.to_string()
@@ -412,7 +412,7 @@ private class Geary.ImapDB.Account : BaseObject {
             Db.Statement stmt = cx.prepare(
                 "INSERT INTO FolderTable (name, parent_id, last_seen_total, last_seen_status_total, "
                 + "uid_validity, uid_next, attributes, unread_count) VALUES (?, ?, ?, ?, ?, ?, ?, ?)");
-            stmt.bind_string(0, path.basename);
+            stmt.bind_string(0, path.name);
             stmt.bind_rowid(1, parent_id);
             stmt.bind_int(2, Numeric.int_floor(properties.select_examine_messages, 0));
             stmt.bind_int(3, Numeric.int_floor(properties.status_messages, 0));
@@ -1589,13 +1589,9 @@ private class Geary.ImapDB.Account : BaseObject {
             );
         }
 
-        // Don't include the root since top-level folders are stored
-        // with no parent.
-        Gee.List<string> parts = path.as_list();
-        parts.remove_at(0);
-
-        folder_id = Db.INVALID_ROWID;
+        string[] parts = path.as_array();
         int64 parent_id = Db.INVALID_ROWID;
+        folder_id = Db.INVALID_ROWID;
 
         foreach (string basename in parts) {
             Db.Statement stmt;
@@ -1658,7 +1654,7 @@ private class Geary.ImapDB.Account : BaseObject {
             parent_id = Db.INVALID_ROWID;
         } else {
             ret = do_fetch_folder_id(
-                cx, path.get_parent(), create, out parent_id, cancellable
+                cx, path.parent, create, out parent_id, cancellable
             );
         }
         return ret;
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index dd3a08fc..f24a6027 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -421,7 +421,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
         return Geary.traverse<FolderPath>(folder_map.keys)
             .filter(p => {
-                FolderPath? path_parent = p.get_parent();
+                FolderPath? path_parent = p.parent;
                 return ((parent == null && path_parent == null) ||
                     (parent != null && path_parent != null && path_parent.equal_to(parent)));
             })
@@ -1283,7 +1283,9 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
                 this.generic_account.remove_folders(to_remove);
 
             // Sort by path length descending, so we always remove children first.
-            removed.sort((a, b) => b.path.get_path_length() - a.path.get_path_length());
+            removed.sort(
+                (a, b) => b.path.as_array().length - a.path.as_array().length
+            );
             foreach (Geary.Folder folder in removed) {
                 try {
                     debug("Locally deleting removed folder %s", folder.to_string());
diff --git a/src/engine/imap/message/imap-mailbox-specifier.vala 
b/src/engine/imap/message/imap-mailbox-specifier.vala
index fb325676..6d7c9160 100644
--- a/src/engine/imap/message/imap-mailbox-specifier.vala
+++ b/src/engine/imap/message/imap-mailbox-specifier.vala
@@ -84,14 +84,14 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable<MailboxSpeci
      * Returns true if the {@link Geary.FolderPath} points to the IMAP Inbox.
      */
     public static bool folder_path_is_inbox(FolderPath path) {
-        return path.is_top_level && is_inbox_name(path.basename);
+        return path.is_top_level && is_inbox_name(path.name);
     }
 
     /**
      * Returns true if the string is the name of the IMAP Inbox.
      *
      * This accounts for IMAP's Inbox name being case-insensitive.  This is only for comparing
-     * folder basenames; this does not account for path delimiters.
+     * folder names; this does not account for path delimiters.
      *
      * See [[http://tools.ietf.org/html/rfc3501#section-5.1]]
      *
@@ -129,19 +129,15 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable<MailboxSpeci
             );
         }
 
-        Gee.List<string> parts = path.as_list();
-        // Don't include the root so that mailboxes do not begin with
-        // the delim.
-        parts.remove_at(0);
-
-        if (parts.size > 1 && delim == null) {
-            throw new ImapError.INVALID(
+        string[] parts = path.as_array();
+        if (parts.length > 1 && delim == null) {
+            throw new ImapError.NOT_SUPPORTED(
                 "Path has more than one part but no delimiter given"
             );
         }
 
         if (String.is_empty_or_whitespace(parts[0])) {
-            throw new ImapError.INVALID(
+            throw new ImapError.NOT_SUPPORTED(
                 "Path contains empty base part: '%s'", path.to_string()
             );
         }
@@ -150,15 +146,14 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable<MailboxSpeci
             is_inbox_name(parts[0]) ? inbox.name : parts[0]
         );
 
-        for (int i = 1; i < parts.size; i++) {
-            string basename = parts[i];
-            if (String.is_empty_or_whitespace(basename)) {
-                throw new ImapError.INVALID(
+        foreach (string name in parts[1:parts.length]) {
+            if (String.is_empty_or_whitespace(name)) {
+                throw new ImapError.NOT_SUPPORTED(
                     "Path contains empty part: '%s'", path.to_string()
                 );
             }
             builder.append(delim);
-            builder.append(basename);
+            builder.append(name);
         }
 
         init(builder.str);
diff --git a/src/engine/imap/transport/imap-client-session.vala 
b/src/engine/imap/transport/imap-client-session.vala
index 0e7910ba..e296c8db 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -533,21 +533,23 @@ public class Geary.Imap.ClientSession : BaseObject {
     public string? get_delimiter_for_path(FolderPath path)
     throws ImapError {
         string? delim = null;
-        Geary.FolderRoot root = path.get_root();
-        if (MailboxSpecifier.folder_path_is_inbox(root)) {
+
+        FolderRoot root = (FolderRoot) path.get_root();
+        if (root.inbox.equal_to(path) ||
+            root.inbox.is_descendant(path)) {
             delim = this.inbox.delim;
         } else {
-            Namespace? ns = this.namespaces.get(root.basename);
+            Namespace? ns = null;
+            FolderPath? search = path;
+            while (ns == null && search != null) {
+                ns = this.namespaces.get(search.name);
+                search = search.parent;
+            }
             if (ns == null) {
-                // Folder's root doesn't exist as a namespace, so try
-                // the empty namespace.
-                ns = this.namespaces.get("");
-                if (ns == null) {
-                    // If that doesn't exist, fall back to the default
-                    // personal namespace
-                    ns = this.personal_namespaces[0];
-                }
+                // fall back to the default personal namespace
+                ns = this.personal_namespaces[0];
             }
+
             delim = ns.delim;
         }
         return delim;
diff --git a/test/engine/api/geary-folder-path-test.vala b/test/engine/api/geary-folder-path-test.vala
index 5d3ef675..c1c16643 100644
--- a/test/engine/api/geary-folder-path-test.vala
+++ b/test/engine/api/geary-folder-path-test.vala
@@ -17,10 +17,12 @@ public class Geary.FolderPathTest : TestCase {
         add_test("get_child_from_child", get_child_from_child);
         add_test("root_is_root", root_is_root);
         add_test("child_is_not_root", root_is_root);
-        add_test("as_list", as_list);
+        add_test("as_array", as_array);
         add_test("is_top_level", is_top_level);
+        add_test("path_to_string", path_to_string);
         add_test("path_parent", path_parent);
         add_test("path_equal", path_equal);
+        add_test("path_hash", path_hash);
         add_test("path_compare", path_compare);
         add_test("path_compare_normalised", path_compare_normalised);
     }
@@ -35,15 +37,15 @@ public class Geary.FolderPathTest : TestCase {
 
     public void get_child_from_root() throws GLib.Error {
         assert_string(
-            ">test",
-            this.root.get_child("test").to_string()
+            "test",
+            this.root.get_child("test").name
         );
     }
 
     public void get_child_from_child() throws GLib.Error {
         assert_string(
-            ">test1>test2",
-            this.root.get_child("test1").get_child("test2").to_string()
+            "test2",
+            this.root.get_child("test1").get_child("test2").name
         );
     }
 
@@ -55,17 +57,32 @@ public class Geary.FolderPathTest : TestCase {
         assert_false(this.root.get_child("test").is_root);
     }
 
-    public void as_list() throws GLib.Error {
-        assert_true(this.root.as_list().size == 1, "Root list");
+    public void as_array() throws GLib.Error {
+        assert_true(this.root.as_array().length == 0, "Root list");
         assert_int(
-            2,
-            this.root.get_child("test").as_list().size,
-            "Child list size"
+            1,
+            this.root.get_child("test").as_array().length,
+            "Child array length"
         );
         assert_string(
             "test",
-            this.root.get_child("test").as_list()[1],
-            "Child list contents"
+            this.root.get_child("test").as_array()[0],
+            "Child array contents"
+        );
+        assert_int(
+            2,
+            this.root.get_child("test1").get_child("test2").as_array().length,
+            "Descendent array length"
+        );
+        assert_string(
+            "test1",
+            this.root.get_child("test1").get_child("test2").as_array()[0],
+            "Descendent first child"
+        );
+        assert_string(
+            "test2",
+            this.root.get_child("test1").get_child("test2").as_array()[1],
+            "Descendent second child"
         );
     }
 
@@ -81,15 +98,24 @@ public class Geary.FolderPathTest : TestCase {
         );
     }
 
+    public void path_to_string() throws GLib.Error {
+        assert_string(">", this.root.to_string());
+        assert_string(">test", this.root.get_child("test").to_string());
+        assert_string(
+            ">test1>test2",
+            this.root.get_child("test1").get_child("test2").to_string()
+        );
+    }
+
     public void path_parent() throws GLib.Error {
-        assert_null(this.root.get_parent(), "Root parent");
+        assert_null(this.root.parent, "Root parent");
         assert_string(
             "",
-            this.root.get_child("test").get_parent().basename,
+            this.root.get_child("test").parent.name,
             "Root child parent");
         assert_string(
             "test1",
-            this.root.get_child("test1").get_child("test2").get_parent().basename,
+            this.root.get_child("test1").get_child("test2").parent.name,
             "Child parent");
     }
 
@@ -110,6 +136,17 @@ public class Geary.FolderPathTest : TestCase {
         );
     }
 
+    public void path_hash() throws GLib.Error {
+        assert_true(
+            this.root.hash() !=
+            this.root.get_child("test").hash()
+        );
+        assert_true(
+            this.root.get_child("test1").hash() !=
+            this.root.get_child("test2").hash()
+        );
+    }
+
     public void path_compare() throws GLib.Error {
         assert_int(0, this.root.compare_to(this.root), "Root equality");
         assert_int(0,
diff --git a/test/engine/imap-db/imap-db-account-test.vala b/test/engine/imap-db/imap-db-account-test.vala
index 27c33de8..d70e44e5 100644
--- a/test/engine/imap-db/imap-db-account-test.vala
+++ b/test/engine/imap-db/imap-db-account-test.vala
@@ -157,7 +157,7 @@ class Geary.ImapDB.AccountTest : TestCase {
 
         Folder test1 = traverse(result).first();
         assert_int(1, result.size, "Base folder not listed");
-        assert_string("test1", test1.get_path().basename, "Base folder name");
+        assert_string("test1", test1.get_path().name, "Base folder name");
 
         this.account.list_folders_async.begin(
             test1.get_path(),
@@ -168,7 +168,7 @@ class Geary.ImapDB.AccountTest : TestCase {
 
         Folder test2 = traverse(result).first();
         assert_int(1, result.size, "Child folder not listed");
-        assert_string("test2", test2.get_path().basename, "Child folder name");
+        assert_string("test2", test2.get_path().name, "Child folder name");
 
         this.account.list_folders_async.begin(
             test2.get_path(),
@@ -179,7 +179,7 @@ class Geary.ImapDB.AccountTest : TestCase {
 
         Folder test3 = traverse(result).first();
         assert_int(1, result.size, "Grandchild folder not listed");
-        assert_string("test3", test3.get_path().basename, "Grandchild folder name");
+        assert_string("test3", test3.get_path().name, "Grandchild folder name");
     }
 
     public void delete_folder() throws GLib.Error {
@@ -263,7 +263,7 @@ class Geary.ImapDB.AccountTest : TestCase {
 
         Folder? result = this.account.fetch_folder_async.end(async_result());
         assert_non_null(result);
-        assert_string("test1", result.get_path().basename);
+        assert_string("test1", result.get_path().name);
     }
 
     public void fetch_child_folder() throws GLib.Error {
@@ -282,7 +282,7 @@ class Geary.ImapDB.AccountTest : TestCase {
 
         Folder? result = this.account.fetch_folder_async.end(async_result());
         assert_non_null(result);
-        assert_string("test2", result.get_path().basename);
+        assert_string("test2", result.get_path().name);
     }
 
     public void fetch_nonexistent_folder() throws GLib.Error {



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