[geary/wip/181-special-folder-dupes: 3/3] Revamp Geary.FolderPath implementation
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/181-special-folder-dupes: 3/3] Revamp Geary.FolderPath implementation
- Date: Mon, 14 Jan 2019 06:34:44 +0000 (UTC)
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]
