[geary/geary-0.13] Merge branch 'wip/224-subfolders-missing' into 'master'
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/geary-0.13] Merge branch 'wip/224-subfolders-missing' into 'master'
- Date: Tue, 19 Feb 2019 11:08:35 +0000 (UTC)
commit f99f31c7047154aac473d54849871e3746776f25
Author: Michael Gratton <mike vee net>
Date: Tue Feb 19 11:07:42 2019 +0000
Merge branch 'wip/224-subfolders-missing' into 'master'
Subfolders missing
Closes #224
See merge request GNOME/geary!118
(cherry picked from commit 25887e53505cd1f2c56f735b15eb5ab52daf5f76)
3aebb98c Tidy up how accounts pass sorted sets of folders around
af676e05 Fix FolderPath mis-sorting doubly-disjoint paths
src/client/application/geary-controller.vala | 19 ++-
src/engine/api/geary-account.vala | 77 ++++++-----
src/engine/api/geary-folder-path.vala | 90 +++++--------
.../imap-engine/imap-engine-generic-account.vala | 40 +++---
.../imap-engine/imap-engine-revokable-move.vala | 11 +-
test/engine/api/geary-folder-path-test.vala | 141 ++++++++++++++++-----
6 files changed, 231 insertions(+), 147 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/api/geary-folder-path.vala b/src/engine/api/geary-folder-path.vala
index ac91ce92..2dafabcf 100644
--- a/src/engine/api/geary-folder-path.vala
+++ b/src/engine/api/geary-folder-path.vala
@@ -36,6 +36,19 @@ public class Geary.FolderPath :
/** The base name of this folder, excluding parents. */
public string name { get; private set; }
+ /** The number of children under the root in this path. */
+ public uint length {
+ get {
+ uint length = 0;
+ FolderPath parent = this.parent;
+ while (parent != null) {
+ length++;
+ parent = parent.parent;
+ }
+ return length;
+ }
+ }
+
/**
* Whether this path is lexiographically case-sensitive.
*
@@ -201,37 +214,7 @@ public class Geary.FolderPath :
/** {@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;
+ return this.compare_internal(other, true, false) == 0;
}
/**
@@ -259,23 +242,29 @@ public class Geary.FolderPath :
private int compare_internal(FolderPath other,
bool allow_case_sensitive,
bool normalize) {
- if (this == other)
+ 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;
- }
+ int a_len = (int) this.length;
+ int b_len = (int) other.length;
+ if (a_len != b_len) {
+ return a_len - b_len;
}
- // Compare the common-length prefixes of both
- while (a != null && b != null) {
+ return compare_names(this, other, allow_case_sensitive, normalize);
+ }
+
+ private static int compare_names(FolderPath a, FolderPath b,
+ bool allow_case_sensitive,
+ bool normalize) {
+ int cmp = 0;
+ if (a.parent != null && b.parent != null) {
+ cmp = compare_names(
+ a.parent, b.parent, allow_case_sensitive, normalize
+ );
+ }
+ if (cmp == 0) {
string a_name = a.name;
string b_name = b.name;
@@ -291,18 +280,9 @@ public class Geary.FolderPath :
b_name = b_name.casefold();
}
- int result = a_name.collate(b_name);
- if (result != 0) {
- return result;
- }
-
- a = a.parent;
- b = b.parent;
+ return strcmp(a_name, b_name);
}
-
- // paths up to the min element count are equal, shortest path
- // is less-than, otherwise equal paths
- return this.path.length - other.path.length;
+ return cmp;
}
}
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)
diff --git a/test/engine/api/geary-folder-path-test.vala b/test/engine/api/geary-folder-path-test.vala
index 18db7308..eecfe81f 100644
--- a/test/engine/api/geary-folder-path-test.vala
+++ b/test/engine/api/geary-folder-path-test.vala
@@ -25,6 +25,7 @@ public class Geary.FolderPathTest : TestCase {
add_test("path_hash", path_hash);
add_test("path_compare", path_compare);
add_test("path_compare_normalised", path_compare_normalised);
+ add_test("distinct_roots_compare", distinct_roots_compare);
}
public override void set_up() {
@@ -156,94 +157,174 @@ public class Geary.FolderPathTest : TestCase {
public void path_compare() throws GLib.Error {
assert_int(0, this.root.compare_to(this.root), "Root equality");
assert_int(0,
- this.root.get_child("test").compare_to(this.root.get_child("test")),
+ this.root.get_child("a").compare_to(this.root.get_child("a")),
"Equal child comparison"
);
+ // a is less than b
assert_int(
-1,
- this.root.get_child("test1").compare_to(this.root.get_child("test2")),
+ this.root.get_child("a").compare_to(this.root.get_child("b")),
"Greater than child comparison"
);
+
+ // b is greater than than a
assert_int(
1,
- this.root.get_child("test2").compare_to(this.root.get_child("test1")),
+ this.root.get_child("b").compare_to(this.root.get_child("a")),
"Less than child comparison"
);
+ assert_int(
+ 1,
+ this.root.get_child("a").get_child("test")
+ .compare_to(this.root.get_child("a")),
+ "Greater than descendant"
+ );
assert_int(
-1,
- this.root.get_child("test1").get_child("test")
- .compare_to(this.root.get_child("test2").get_child("test")),
- "Greater than disjoint parents"
+ this.root.get_child("a")
+ .compare_to(this.root.get_child("a").get_child("test")),
+ "Less than descendant"
);
+
assert_int(
- 1,
- this.root.get_child("test2").get_child("test")
- .compare_to(this.root.get_child("test1").get_child("test")),
- "Less than disjoint parents"
+ 0,
+ this.root.get_child("a").get_child("b")
+ .compare_to(this.root.get_child("a").get_child("b")),
+ "N-path equality"
);
+ assert_int(
+ -1,
+ this.root.get_child("a").get_child("test")
+ .compare_to(this.root.get_child("b").get_child("test")),
+ "Greater than disjoint paths"
+ );
assert_int(
1,
- this.root.get_child("test1").get_child("test")
- .compare_to(this.root.get_child("test1")),
- "Greater than descendant"
+ this.root.get_child("b").get_child("test")
+ .compare_to(this.root.get_child("a").get_child("test")),
+ "Less than disjoint paths"
);
+
assert_int(
-1,
- this.root.get_child("test1")
- .compare_to(this.root.get_child("test1").get_child("test")),
- "Less than descendant"
+ this.root.get_child("a").get_child("d")
+ .compare_to(this.root.get_child("b").get_child("c")),
+ "Greater than double disjoint"
+ );
+ assert_int(
+ 1,
+ this.root.get_child("b").get_child("c")
+ .compare_to(this.root.get_child("a").get_child("d")),
+ "Less than double disjoint"
);
+
}
public void path_compare_normalised() throws GLib.Error {
assert_int(0, this.root.compare_normalized_ci(this.root), "Root equality");
assert_int(0,
- this.root.get_child("test")
- .compare_normalized_ci(this.root.get_child("test")),
+ this.root.get_child("a").compare_normalized_ci(this.root.get_child("a")),
"Equal child comparison"
);
+ // a is less than b
assert_int(
-1,
- this.root.get_child("test1")
- .compare_normalized_ci(this.root.get_child("test2")),
+ this.root.get_child("a").compare_normalized_ci(this.root.get_child("b")),
"Greater than child comparison"
);
+
+ // b is greater than than a
assert_int(
1,
- this.root.get_child("test2")
- .compare_normalized_ci(this.root.get_child("test1")),
+ this.root.get_child("b").compare_normalized_ci(this.root.get_child("a")),
"Less than child comparison"
);
assert_int(
-1,
- this.root.get_child("test1").get_child("test")
- .compare_normalized_ci(this.root.get_child("test2").get_child("test")),
+ this.root.get_child("a").get_child("test")
+ .compare_normalized_ci(this.root.get_child("b").get_child("test")),
"Greater than disjoint parents"
);
assert_int(
1,
- this.root.get_child("test2").get_child("test")
- .compare_normalized_ci(this.root.get_child("test1").get_child("test")),
+ this.root.get_child("b").get_child("test")
+ .compare_normalized_ci(this.root.get_child("a").get_child("test")),
"Less than disjoint parents"
);
assert_int(
1,
- this.root.get_child("test1").get_child("test")
- .compare_normalized_ci(this.root.get_child("test1")),
+ this.root.get_child("a").get_child("test")
+ .compare_normalized_ci(this.root.get_child("a")),
+ "Greater than descendant"
+ );
+ assert_int(
+ -1,
+ this.root.get_child("a")
+ .compare_normalized_ci(this.root.get_child("a").get_child("test")),
+ "Less than descendant"
+ );
+ }
+
+ public void distinct_roots_compare() throws GLib.Error {
+ assert_int(0, this.root.compare_to(new FolderRoot(false)), "Root equality");
+ assert_int(0,
+ this.root.get_child("a").compare_to(new FolderRoot(false).get_child("a")),
+ "Equal child comparison"
+ );
+
+ // a is less than b
+ assert_int(
+ -1,
+ this.root.get_child("a").compare_to(new FolderRoot(false).get_child("b")),
+ "Greater than child comparison"
+ );
+
+ // b is greater than than a
+ assert_int(
+ 1,
+ this.root.get_child("b").compare_to(new FolderRoot(false).get_child("a")),
+ "Less than child comparison"
+ );
+
+ assert_int(
+ 1,
+ this.root.get_child("a").get_child("test")
+ .compare_to(new FolderRoot(false).get_child("a")),
"Greater than descendant"
);
assert_int(
-1,
- this.root.get_child("test1")
- .compare_normalized_ci(this.root.get_child("test1").get_child("test")),
+ this.root.get_child("a")
+ .compare_to(new FolderRoot(false).get_child("a").get_child("test")),
"Less than descendant"
);
+
+ assert_int(
+ 0,
+ this.root.get_child("a").get_child("b")
+ .compare_to(new FolderRoot(false).get_child("a").get_child("b")),
+ "N-path equality"
+ );
+
+ assert_int(
+ -1,
+ this.root.get_child("a").get_child("a")
+ .compare_to(new FolderRoot(false).get_child("b").get_child("b")),
+ "Greater than double disjoint"
+ );
+ assert_int(
+ 1,
+ this.root.get_child("b").get_child("a")
+ .compare_to(new FolderRoot(false).get_child("a").get_child("a")),
+ "Less than double disjoint"
+ );
+
}
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
