Commit f99f31c7 authored by Michael Gratton's avatar Michael Gratton 🤞

Merge branch 'wip/224-subfolders-missing' into 'master'

Subfolders missing

Closes #224

See merge request !118

(cherry picked from commit 25887e53)

3aebb98c Tidy up how accounts pass sorted sets of folders around
af676e05 Fix FolderPath mis-sorting doubly-disjoint paths
parent 2a91b8dd
Pipeline #60594 failed with stages
in 42 minutes and 43 seconds
......@@ -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;
......
......@@ -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);
}
......
......@@ -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;
}
}
......
......@@ -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);
......
......@@ -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)
......
......@@ -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"
);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment