nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make Geary.App.ConversationSet unit testable, add unit tests.

Browse source 
* src/engine/app/conversation-monitor/app-conversation-set.vala
  (ConversationSet.add_all_emails_async): Replace monitor arg with a
  multiset of email paths, update call site to do the path lookup on the
  caller's side instead. Add unit tests.
  (ConversationSet.merge_conversations_async): Remove monitor arg with a
  multiset of email paths, get paths for emails to be merged from the
  conversations themselves, since they should be up to date.
  (Conversation.remove_all_emails_by_identifier): Add unit tests.
This commit is contained in:
Michael James Gratton 2017-12-07 10:25:14 +11:00
parent 57f40ffec3
commit 5c1aecb685
6 changed files with 548 additions and 52 deletions
Download patch file Download diff file Expand all files Collapse all files

17
src/engine/app/app-conversation-monitor.vala
View file

@ -567,8 +567,21 @@ public class Geary.App.ConversationMonitor : BaseObject {
Gee.MultiMap<Geary.App.Conversation, Geary.Email>? appended = null;
Gee.Collection<Conversation>? removed_due_to_merge = null;
try {
yield conversations.add_all_emails_async(job.emails.values, this, folder.path, out added, out appended,
out removed_due_to_merge, null);
// Get known paths for all emails
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath>? email_paths =
yield this.folder.account.get_containing_folders_async(
job.emails.keys, null
);
// Add them to the conversation set
yield this.conversations.add_all_emails_async(
job.emails.values,
email_paths,
this.folder,
out added,
out appended,
out removed_due_to_merge,
null);
} catch (Error err) {
debug("Unable to add emails to conversation: %s", err.message);

13
src/engine/app/app-conversation.vala
View file

@ -43,6 +43,13 @@ public class Geary.App.Conversation : BaseObject {
/** Folder from which the conversation originated. */
public Folder base_folder { get; private set; }
/** Cache of paths associated with each email */
internal Gee.HashMultiMap<Geary.EmailIdentifier,Geary.FolderPath> path_map {
get;
private set;
default = new Gee.HashMultiMap< Geary.EmailIdentifier,Geary.FolderPath>();
}
private Gee.HashMultiSet<RFC822.MessageID> message_ids = new Gee.HashMultiSet<RFC822.MessageID>();
private int convnum;
@ -59,11 +66,7 @@ public class Geary.App.Conversation : BaseObject {
Geary.Email.compare_recv_date_ascending);
private Gee.SortedSet<Email> recv_date_descending = new Gee.TreeSet<Email>(
Geary.Email.compare_recv_date_descending);
// by storing all paths for each EmailIdentifier, can lookup without blocking
private Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath> path_map = new Gee.HashMultiMap<
Geary.EmailIdentifier, Geary.FolderPath>();
/**
* Fired when email has been added to this conversation.
*/

115
src/engine/app/conversation-monitor/app-conversation-set.vala
View file

@ -84,8 +84,9 @@ private class Geary.App.ConversationSet : BaseObject {
* NOTE: Do not call this method if get_associated_conversations() returns a Collection with
* a size greater than one. That indicates the Conversations *must* be merged before adding.
*/
private Conversation? add_email(Geary.Email email, ConversationMonitor monitor,
Geary.FolderPath? preferred_folder_path, Gee.Collection<Geary.FolderPath>? known_paths,
private Conversation? add_email(Geary.Email email,
Folder base_folder,
Gee.Collection<Geary.FolderPath>? known_paths,
out bool added_conversation) {
added_conversation = false;
@ -100,7 +101,7 @@ private class Geary.App.ConversationSet : BaseObject {
conversation = Collection.get_first<Conversation>(associated);
if (conversation == null) {
conversation = new Conversation(monitor.folder);
conversation = new Conversation(base_folder);
_conversations.add(conversation);
added_conversation = true;
@ -126,25 +127,38 @@ private class Geary.App.ConversationSet : BaseObject {
logical_message_id_map.set(ancestor, conversation);
}
}
public async void add_all_emails_async(Gee.Collection<Geary.Email> emails,
ConversationMonitor monitor, Geary.FolderPath? preferred_folder_path,
/**
* Adds a set of emails to conversations in this set.
*
* This method will create and/or merge conversations as
* needed. The collection `emails` contains the messages to be
* added, and for each email in the collection, there should be an
* entry in `id_to_paths` that indicates the folders each message
* is known to belong to. The folder `base_folder` is the base
* folder for the conversation monitor that owns this set.
*
* The three collections returned include any conversation that
* were created, any that had email appended to them (and the
* messages that were appended), and any that were removed due to
* being merged into another.
*/
public async void add_all_emails_async(
Gee.Collection<Geary.Email> emails,
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath>? id_to_paths,
Folder base_folder,
out Gee.Collection<Conversation> added,
out Gee.MultiMap<Conversation, Geary.Email> appended,
out Gee.Collection<Conversation> removed_due_to_merge,
Cancellable? cancellable) throws Error {
// Get known paths for all emails
Gee.Map<Geary.EmailIdentifier, Geary.Email>? id_map = Email.emails_to_map(emails);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath>? id_to_paths = null;
if (id_map != null) {
id_to_paths = yield monitor.folder.account.get_containing_folders_async(id_map.keys,
cancellable);
}
Gee.HashSet<Conversation> _added = new Gee.HashSet<Conversation>();
Gee.HashMultiMap<Conversation, Geary.Email> _appended
= new Gee.HashMultiMap<Conversation, Geary.Email>();
Gee.HashSet<Conversation> _removed_due_to_merge = new Gee.HashSet<Conversation>();
Cancellable? cancellable)
throws Error {
Gee.HashSet<Conversation> _added =
new Gee.HashSet<Conversation>();
Gee.HashMultiMap<Conversation, Geary.Email> _appended =
new Gee.HashMultiMap<Conversation, Geary.Email>();
Gee.HashSet<Conversation> _removed_due_to_merge =
new Gee.HashSet<Conversation>();
foreach (Geary.Email email in emails) {
Gee.Set<Conversation> associated = get_associated_conversations(email);
if (associated.size > 1) {
@ -157,14 +171,15 @@ private class Geary.App.ConversationSet : BaseObject {
// By doing this first, it prevents ConversationSet getting itself into a bad state
// where more than one Conversation thinks it "owns" a Message-ID
debug("Merging %d conversations due new email associating with all...", associated.size);
// Note that this call will modify the List so it only holds the to-be-axed
// Conversations
Gee.Set<Geary.Email> moved_email = new Gee.HashSet<Geary.Email>();
Conversation dest = yield merge_conversations_async(monitor, associated, moved_email,
cancellable);
Conversation dest = yield merge_conversations_async(
associated, moved_email, cancellable
);
assert(!associated.contains(dest));
// remove the remaining conversations from the added/appended Collections
_added.remove_all(associated);
foreach (Conversation removed_conversation in associated)
@ -182,13 +197,15 @@ private class Geary.App.ConversationSet : BaseObject {
// Nasty ol' Email won't cause problems now -- but let's check anyway!
assert(get_associated_conversations(email).size <= 1);
}
bool added_conversation;
Conversation? conversation = add_email(
email, monitor, preferred_folder_path,
email,
base_folder,
(id_to_paths != null) ? id_to_paths.get(email.id) : null,
out added_conversation);
out added_conversation
);
if (conversation == null)
continue;
@ -199,19 +216,21 @@ private class Geary.App.ConversationSet : BaseObject {
_appended.set(conversation, email);
}
}
added = _added;
appended = _appended;
removed_due_to_merge = _removed_due_to_merge;
}
// This method will remove the destination (merged) Conversation from the List and return it
// as the result, along with a Collection of email that must be merged into it
private async Conversation merge_conversations_async(ConversationMonitor monitor,
Gee.Set<Conversation> conversations, Gee.Set<Geary.Email> moved_email,
Cancellable? cancellable) throws Error {
private async Conversation
merge_conversations_async(Gee.Set<Conversation> conversations,
Gee.Set<Geary.Email> moved_email,
Cancellable? cancellable)
throws Error {
assert(conversations.size > 0);
// find the largest conversation and merge the others into it
Conversation? dest = null;
foreach (Conversation conversation in conversations) {
@ -223,10 +242,19 @@ private class Geary.App.ConversationSet : BaseObject {
// conversations merged into it
bool removed = conversations.remove(dest);
assert(removed);
foreach (Conversation conversation in conversations)
moved_email.add_all(conversation.get_emails(Conversation.Ordering.NONE));
Gee.MultiMap<Geary.EmailIdentifier,Geary.FolderPath>? id_to_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier,Geary.FolderPath>();
foreach (Conversation conversation in conversations) {
foreach (EmailIdentifier id in conversation.path_map.get_keys()) {
moved_email.add(conversation.get_email_by_id(id));
foreach (FolderPath path in conversation.path_map.get(id)) {
id_to_paths.set(id, path);
}
}
}
// convert total sum of Emails to move into map of ID -> Email
Gee.Map<Geary.EmailIdentifier, Geary.Email>? id_map = Geary.Email.emails_to_map(moved_email);
// there better be some Email here, otherwise things are really hosed
@ -243,15 +271,12 @@ private class Geary.App.ConversationSet : BaseObject {
assert(removed_conversations.size == conversations.size);
foreach (Conversation conversation in conversations)
assert(removed_conversations.contains(conversation));
// Get known paths for all emails being moved
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath>? id_to_paths =
yield monitor.folder.account.get_containing_folders_async(id_map.keys, cancellable);
// now add all that email back to the destination Conversation
foreach (Geary.Email moved in moved_email)
add_email_to_conversation(dest, moved, (id_to_paths != null) ? id_to_paths.get(moved.id) : null);
foreach (Geary.Email moved in moved_email) {
add_email_to_conversation(dest, moved, id_to_paths.get(moved.id));
}
return dest;
}

1
test/CMakeLists.txt
View file

@ -12,6 +12,7 @@ set(TEST_ENGINE_SRC
engine/api/geary-folder-test.vala
engine/api/geary-folder-path-test.vala
engine/app/app-conversation-test.vala
engine/app/app-conversation-set-test.vala
engine/imap/command/imap-create-command-test.vala
engine/imap/response/imap-namespace-response-test.vala
engine/imap/transport/imap-deserializer-test.vala

453
test/engine/app/app-conversation-set-test.vala Normal file
View file

@ -0,0 +1,453 @@
/*
* Copyright 2017 Michael Gratton <mike@vee.net>
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
class Geary.App.ConversationSetTest : Gee.TestCase {
ConversationSet? test = null;
Folder? base_folder = null;
public ConversationSetTest() {
base("Geary.App.ConversationSetTest");
add_test("add_all_basic", add_all_basic);
add_test("add_all_duplicate", add_all_duplicate);
add_test("add_all_append_descendants", add_all_append_descendants);
add_test("add_all_append_ancestors", add_all_append_ancestors);
add_test("add_all_merge", add_all_merge);
add_test("remove_all_removed", remove_all_removed);
add_test("remove_all_trimmed", remove_all_trimmed);
}
public override void set_up() {
this.test = new ConversationSet();
this.base_folder = new MockFolder(
null,
null,
new MockFolderRoot("test"),
SpecialFolderType.NONE,
null
);
}
public void add_all_basic() {
Email e1 = new Email(new MockEmailIdentifer(1));
Email e2 = new Email(new MockEmailIdentifer(2));
Gee.LinkedList<Email> email = new Gee.LinkedList<Email>();
email.add(e1);
email.add(e2);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath> email_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath>();
email_paths.set(e1.id, this.base_folder.path);
email_paths.set(e2.id, this.base_folder.path);
Gee.Collection<Conversation>? added = null;
Gee.MultiMap<Conversation,Email>? appended = null;
Gee.Collection<Conversation>? removed = null;
this.test.add_all_emails_async.begin(
email,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 2);
assert(this.test.get_email_count() == 2);
assert(added != null);
assert(appended != null);
assert(removed != null);
// Work out which collection in the collection corresponds to which
assert(added.size == 2);
Conversation? c1 = null;
Conversation? c2 = null;
foreach (Conversation convo in added) {
if (convo.get_email_by_id(e1.id) != null) {
c1 = convo;
} else if (convo.get_email_by_id(e2.id) != null) {
c2 = convo;
}
}
assert(c1 != null);
assert(c2 != null);
assert(appended.size == 0);
assert(removed.is_empty);
}
public void add_all_duplicate() {
Email e1 = setup_email(1);
Gee.LinkedList<Email> email = new Gee.LinkedList<Email>();
email.add(e1);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath> email_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath>();
email_paths.set(e1.id, this.base_folder.path);
// Pass 1: Duplicate in same input collection
Gee.Collection<Conversation>? added = null;
Gee.MultiMap<Conversation,Email>? appended = null;
Gee.Collection<Conversation>? removed = null;
this.test.add_all_emails_async.begin(
email,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 1);
assert(this.test.get_email_count() == 1);
assert(added.size == 1);
assert(Geary.Collection.get_first(added).get_email_by_id(e1.id) == e1);
assert(appended.size == 0);
assert(removed.is_empty);
// Pass 2: Duplicate being re-added
added = null;
appended = null;
removed = null;
this.test.add_all_emails_async.begin(
email,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 1);
assert(this.test.get_email_count() == 1);
assert(added.is_empty);
assert(appended.size == 0);
assert(removed.is_empty);
}
public void add_all_append_descendants() {
Email e1 = setup_email(1);
Email e2 = setup_email(2, e1);
// Pass 1: Append two at the same time
Gee.LinkedList<Email> emails = new Gee.LinkedList<Email>();
emails.add(e1);
emails.add(e2);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath> email_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath>();
email_paths.set(e1.id, this.base_folder.path);
email_paths.set(e2.id, new MockFolderRoot("other"));
Gee.Collection<Conversation>? added = null;
Gee.MultiMap<Conversation,Email>? appended = null;
Gee.Collection<Conversation>? removed = null;
this.test.add_all_emails_async.begin(
emails,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 1);
assert(this.test.get_email_count() == 2);
assert(added.size == 1);
Conversation convo1 = Geary.Collection.get_first(added);
assert(convo1.get_email_by_id(e1.id) == e1);
assert(convo1.get_email_by_id(e2.id) == e2);
assert(appended.size == 0);
assert(removed.is_empty);
// Pass 2: Append one to an existing convo
Email e3 = setup_email(3, e1);
emails.clear();
email_paths.clear();
emails.add(e3);
email_paths.set(e3.id, this.base_folder.path);
added = null;
appended = null;
removed = null;
this.test.add_all_emails_async.begin(
emails,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 1);
assert(this.test.get_email_count() == 3);
assert(added.is_empty);
assert(appended.size == 1);
Conversation convo2 = Geary.Collection.get_first(appended.get_keys());
assert(convo2.get_email_by_id(e1.id) == e1);
assert(convo2.get_email_by_id(e2.id) == e2);
assert(convo2.get_email_by_id(e3.id) == e3);
assert(appended.contains(convo2) == true);
assert(appended.get(convo2).contains(e1) != true);
assert(appended.get(convo2).contains(e2) != true);
assert(appended.get(convo2).contains(e3) == true);
assert(removed.is_empty);
}
public void add_all_append_ancestors() {
Email e1 = setup_email(1);
Email e2 = setup_email(2, e1);
add_email_to_test_set(e2);
Gee.LinkedList<Email> emails = new Gee.LinkedList<Email>();
emails.add(e1);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath> email_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath>();
email_paths.set(e1.id, this.base_folder.path);
Gee.Collection<Conversation>? added = null;
Gee.MultiMap<Conversation,Email>? appended = null;
Gee.Collection<Conversation>? removed = null;
this.test.add_all_emails_async.begin(
emails,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 1);
assert(this.test.get_email_count() == 2);
assert(added.is_empty);
assert(appended.size == 1);
Conversation convo = Geary.Collection.get_first(appended.get_keys());
assert(convo.get_email_by_id(e1.id) == e1);
assert(convo.get_email_by_id(e2.id) == e2);
assert(appended.contains(convo) == true);
assert(appended.get(convo).contains(e1) == true);
assert(removed.is_empty);
}
public void add_all_merge() {
Email e1 = setup_email(1);
add_email_to_test_set(e1);
Email e2 = setup_email(2, e1);
Email e3 = setup_email(3, e2);
add_email_to_test_set(e3);
assert(this.test.size == 2);
assert(this.test.get_email_count() == 2);
Gee.LinkedList<Email> emails = new Gee.LinkedList<Email>();
emails.add(e2);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath> email_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath>();
email_paths.set(e2.id, this.base_folder.path);
Gee.Collection<Conversation>? added = null;
Gee.MultiMap<Conversation,Email>? appended = null;
Gee.Collection<Conversation>? removed = null;
this.test.add_all_emails_async.begin(
emails,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
assert(this.test.size == 1);
assert(this.test.get_email_count() == 3);
Conversation convo = this.test.get_by_email_identifier(e1.id);
assert(convo.get_email_by_id(e1.id) == e1);
assert(convo.get_email_by_id(e2.id) == e2);
assert(convo.get_email_by_id(e3.id) == e3);
assert(appended.size == 2);
assert(appended.get(convo) != null);
assert(appended.get(convo).contains(e2) == true);
assert(appended.get(convo).contains(e3) == true);
assert(added.is_empty);
assert(removed.size == 1);
}
public void remove_all_removed() {
Email e1 = setup_email(1);
add_email_to_test_set(e1);
Conversation convo = this.test.get_by_email_identifier(e1.id);
Gee.LinkedList<EmailIdentifier> ids =
new Gee.LinkedList<EmailIdentifier>();
ids.add(e1.id);
Gee.Collection<Conversation>? removed = null;
Gee.MultiMap<Conversation,Email>? trimmed = null;
this.test.remove_all_emails_by_identifier(
ids, out removed, out trimmed
);
assert(this.test.size == 0);
assert(this.test.get_email_count() == 0);
assert(removed != null);
assert(trimmed != null);
assert(removed.contains(convo) == true);
assert(trimmed.size == 0);
}
public void remove_all_trimmed() {
Email e1 = setup_email(1);
add_email_to_test_set(e1);
Email e2 = setup_email(2, e1);
add_email_to_test_set(e2);
Conversation convo = this.test.get_by_email_identifier(e1.id);
Gee.LinkedList<EmailIdentifier> ids =
new Gee.LinkedList<EmailIdentifier>();
ids.add(e1.id);
Gee.Collection<Conversation>? removed = null;
Gee.MultiMap<Conversation,Email>? trimmed = null;
this.test.remove_all_emails_by_identifier(
ids, out removed, out trimmed
);
assert(this.test.size == 1);
assert(this.test.get_email_count() == 1);
assert(removed != null);
assert(trimmed != null);
assert(removed.is_empty == true);
assert(trimmed.contains(convo) == true);
assert(trimmed.get(convo).contains(e1) == true);
}
private Email setup_email(int id, Email? references = null) {
Email email = new Email(new MockEmailIdentifer(id));
Geary.RFC822.MessageID mid = new Geary.RFC822.MessageID(
"test%d@localhost".printf(id)
);
Geary.RFC822.MessageIDList refs_list = null;
if (references != null) {
refs_list = new Geary.RFC822.MessageIDList.single(
references.message_id
);
}
email.set_full_references(mid, null, refs_list);
return email;
}
private void add_email_to_test_set(Email to_add) {
Gee.LinkedList<Email> emails = new Gee.LinkedList<Email>();
emails.add(to_add);
Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath> email_paths =
new Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath>();
email_paths.set(to_add.id, this.base_folder.path);
Gee.Collection<Conversation>? added = null;
Gee.MultiMap<Conversation,Email>? appended = null;
Gee.Collection<Conversation>? removed = null;
this.test.add_all_emails_async.begin(
emails,
email_paths,
this.base_folder,
null,
(obj, ret) => { async_complete(ret); }
);
try {
this.test.add_all_emails_async.end(
async_result(), out added, out appended, out removed
);
} catch (Error error) {
assert_not_reached();
}
}
}

1
test/test-engine.vala
View file

@ -27,6 +27,7 @@ int main(string[] args) {
engine.add_suite(new Geary.IdleManagerTest().get_suite());
engine.add_suite(new Geary.TimeoutManagerTest().get_suite());
engine.add_suite(new Geary.App.ConversationTest().get_suite());
engine.add_suite(new Geary.App.ConversationSetTest().get_suite());
engine.add_suite(new Geary.HTML.UtilTest().get_suite());
engine.add_suite(new Geary.Imap.DeserializerTest().get_suite());
engine.add_suite(new Geary.Imap.CreateCommandTest().get_suite());