From 2e8c3dbbb86651d4c6b7495e3d68700eb099c33b Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Thu, 5 Sep 2013 18:46:38 -0700 Subject: [PATCH] Improved Folder notification of added email Folder now offers an "email-inserted" and "email-locally-inserted" signal to indicate when email has been added (or discovered) but is not added to the top of the message vector. SearchFolder and ConversationMonitor may be able to use this better in the future. --- .../abstract/geary-abstract-account.vala | 4 ++ .../abstract/geary-abstract-folder.vala | 8 ++- src/engine/api/geary-account.vala | 12 ++++ src/engine/api/geary-folder.vala | 47 +++++++++++---- .../imap-db/outbox/smtp-outbox-folder.vala | 1 + .../imap-engine-email-prefetcher.vala | 6 +- .../imap-engine-generic-account.vala | 43 ++++--------- .../imap-engine-generic-folder.vala | 60 +++++++++++++------ .../imap-engine-abstract-list-email.vala | 6 +- .../replay-ops/imap-engine-expunge-email.vala | 4 +- .../replay-ops/imap-engine-fetch-email.vala | 7 ++- .../replay-ops/imap-engine-move-email.vala | 4 +- 12 files changed, 128 insertions(+), 74 deletions(-) diff --git a/src/engine/abstract/geary-abstract-account.vala b/src/engine/abstract/geary-abstract-account.vala index 72e86e93..a7ea9f0b 100644 --- a/src/engine/abstract/geary-abstract-account.vala +++ b/src/engine/abstract/geary-abstract-account.vala @@ -35,6 +35,10 @@ public abstract class Geary.AbstractAccount : BaseObject, Geary.Account { email_appended(folder, ids); } + protected virtual void notify_email_inserted(Geary.Folder folder, Gee.Collection ids) { + email_inserted(folder, ids); + } + protected virtual void notify_email_removed(Geary.Folder folder, Gee.Collection ids) { email_removed(folder, ids); } diff --git a/src/engine/abstract/geary-abstract-folder.vala b/src/engine/abstract/geary-abstract-folder.vala index a9de303e..615d7400 100644 --- a/src/engine/abstract/geary-abstract-folder.vala +++ b/src/engine/abstract/geary-abstract-folder.vala @@ -30,8 +30,12 @@ public abstract class Geary.AbstractFolder : BaseObject, Geary.Folder { email_locally_appended(ids); } - internal virtual void notify_email_discovered(Gee.Collection ids) { - email_discovered(ids); + internal virtual void notify_email_inserted(Gee.Collection ids) { + email_inserted(ids); + } + + internal virtual void notify_email_locally_inserted(Gee.Collection ids) { + email_locally_inserted(ids); } internal virtual void notify_email_removed(Gee.Collection ids) { diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala index a49a725f..a1949607 100644 --- a/src/engine/api/geary-account.vala +++ b/src/engine/api/geary-account.vala @@ -65,6 +65,13 @@ public interface Geary.Account : BaseObject { */ public signal void email_appended(Geary.Folder folder, Gee.Collection ids); + /** + * Fired when emails are inserted to a folder in this account. + * + * @see Folder.email_inserted + */ + public signal void email_inserted(Geary.Folder folder, Gee.Collection ids); + /** * Fired when emails are removed from a folder in this account. */ @@ -131,6 +138,11 @@ public interface Geary.Account : BaseObject { */ protected abstract void notify_email_appended(Geary.Folder folder, Gee.Collection ids); + /** + * Signal notification method for subclasses to use. + */ + protected abstract void notify_email_inserted(Geary.Folder folder, Gee.Collection ids); + /** * Signal notification method for subclasses to use. */ diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index 87116b74..b7ad434b 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -195,15 +195,42 @@ public interface Geary.Folder : BaseObject { /** * Fired when previously unknown messages have been appended to the list of email in the folder. * - * This is similar to {@link email_appended}, but that signal - * lists ''all'' messages appended to the folder. email_locally_appended only reports email that - * have not been seen prior. Hence, an email that is removed from the folder and returned - * later will not be listed here (unless it was removed from the local store in the meantime). + * This is similar to {@link email_appended}, but that signal lists ''all'' messages appended + * to the folder. email_locally_appended only reports email that have not been downloaded + * prior to the database (and not removed permanently since). Hence, an email that is removed + * from the folder and returned later will not be listed here (unless it was removed from the + * local store in the meantime). * * @see email_appended */ public signal void email_locally_appended(Gee.Collection ids); + /** + * Fired when email has been inserted into the list of messages in the folder. + * + * The {@link EmailIdentifier} for all inserted messages is supplied as a signal parameter. + * Inserted messages are not added to the "top" of the vector of messages, but rather into + * the middle or beginning. This can happen for a number of reasons. Newly received messages + * are appended. + * + * @see email_locally_inserted + */ + public signal void email_inserted(Gee.Collection ids); + + /** + * Fired when previously unknown messages have been appended to the list of email in the folder. + * + * This is similar to {@link email_inserted}, but that signal lists ''all'' messages inserted + * to the folder. email_locally_inserted only reports email that have not been downloaded + * prior to the database (and not removed permanently since). Hence, an email that is removed + * from the folder and returned later will not be listed here (unless it was removed from the + * local store in the meantime). + * + * @see email_inserted + * @see email_locally_inserted + */ + public signal void email_locally_inserted(Gee.Collection ids); + /** * Fired when email has been removed (deleted or moved) from the folder. * @@ -237,12 +264,6 @@ public interface Geary.Folder : BaseObject { */ public signal void email_locally_complete(Gee.Collection ids); - /** - * Fired when one or more emails have been discovered (added) to the Folder, but not necessarily - * appended (i.e. old email pulled down due to user request or background fetching). - */ - public signal void email_discovered(Gee.Collection ids); - /** * Fired when the {@link SpecialFolderType} has changed. * @@ -262,6 +283,10 @@ public interface Geary.Folder : BaseObject { protected abstract void notify_email_locally_appended(Gee.Collection ids); + protected abstract void notify_email_inserted(Gee.Collection ids); + + protected abstract void notify_email_locally_inserted(Gee.Collection ids); + protected abstract void notify_email_removed(Gee.Collection ids); protected abstract void notify_email_count_changed(int new_count, CountChangeReason reason); @@ -271,8 +296,6 @@ public interface Geary.Folder : BaseObject { protected abstract void notify_email_locally_complete(Gee.Collection ids); - protected abstract void notify_email_discovered(Gee.Collection ids); - protected abstract void notify_special_folder_type_changed(Geary.SpecialFolderType old_type, Geary.SpecialFolderType new_type); diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala b/src/engine/imap-db/outbox/smtp-outbox-folder.vala index 8ea4dfe6..92a1c46c 100644 --- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala +++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala @@ -296,6 +296,7 @@ private class Geary.SmtpOutboxFolder : Geary.AbstractLocalFolder, Geary.FolderSu list.add(row.outbox_id); notify_email_appended(list); + notify_email_locally_appended(list); notify_email_count_changed(email_count, CountChangeReason.APPENDED); } diff --git a/src/engine/imap-engine/imap-engine-email-prefetcher.vala b/src/engine/imap-engine/imap-engine-email-prefetcher.vala index 33d5efca..eff9e060 100644 --- a/src/engine/imap-engine/imap-engine-email-prefetcher.vala +++ b/src/engine/imap-engine/imap-engine-email-prefetcher.vala @@ -39,7 +39,8 @@ private class Geary.ImapEngine.EmailPrefetcher : Object { folder.opened.connect(on_opened); folder.closed.connect(on_closed); - folder.email_discovered.connect(on_local_expansion); + folder.email_appended.connect(on_local_expansion); + folder.email_inserted.connect(on_local_expansion); } ~EmailPrefetcher() { @@ -48,7 +49,8 @@ private class Geary.ImapEngine.EmailPrefetcher : Object { folder.opened.disconnect(on_opened); folder.closed.disconnect(on_closed); - folder.email_discovered.disconnect(on_local_expansion); + folder.email_appended.disconnect(on_local_expansion); + folder.email_inserted.disconnect(on_local_expansion); } private void on_opened(Geary.Folder.OpenState open_state) { diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 947b287f..eff17725 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -49,47 +49,24 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { base.notify_folders_available_unavailable(available, unavailable); if (available != null) { foreach (Geary.Folder folder in available) { - folder.email_appended.connect(on_folder_email_appended); - folder.email_removed.connect(on_folder_email_removed); - folder.email_locally_complete.connect(on_folder_email_locally_complete); - folder.email_discovered.connect(on_folder_email_discovered); - folder.email_flags_changed.connect(on_folder_email_flags_changed); + folder.email_appended.connect(notify_email_appended); + folder.email_inserted.connect(notify_email_inserted); + folder.email_removed.connect(notify_email_removed); + folder.email_locally_complete.connect(notify_email_locally_complete); + folder.email_flags_changed.connect(notify_email_flags_changed); } } if (unavailable != null) { foreach (Geary.Folder folder in unavailable) { - folder.email_appended.disconnect(on_folder_email_appended); - folder.email_removed.disconnect(on_folder_email_removed); - folder.email_locally_complete.disconnect(on_folder_email_locally_complete); - folder.email_discovered.disconnect(on_folder_email_discovered); - folder.email_flags_changed.disconnect(on_folder_email_flags_changed); + folder.email_appended.disconnect(notify_email_appended); + folder.email_inserted.disconnect(notify_email_inserted); + folder.email_removed.disconnect(notify_email_removed); + folder.email_locally_complete.disconnect(notify_email_locally_complete); + folder.email_flags_changed.disconnect(notify_email_flags_changed); } } } - private void on_folder_email_appended(Geary.Folder folder, Gee.Collection ids) { - notify_email_appended(folder, ids); - } - - private void on_folder_email_removed(Geary.Folder folder, Gee.Collection ids) { - notify_email_removed(folder, ids); - } - - private void on_folder_email_locally_complete(Geary.Folder folder, - Gee.Collection ids) { - notify_email_locally_complete(folder, ids); - } - - private void on_folder_email_discovered(Geary.Folder folder, - Gee.Collection ids) { - notify_email_discovered(folder, ids); - } - - private void on_folder_email_flags_changed(Geary.Folder folder, - Gee.Map flag_map) { - notify_email_flags_changed(folder, flag_map); - } - private void check_open() throws EngineError { if (!open) throw new EngineError.OPEN_REQUIRED("Account %s not opened", to_string()); diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala b/src/engine/imap-engine/imap-engine-generic-folder.vala index 1b4003d1..b49d46ca 100644 --- a/src/engine/imap-engine/imap-engine-generic-folder.vala +++ b/src/engine/imap-engine/imap-engine-generic-folder.vala @@ -232,7 +232,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde Gee.HashSet removed_uids = new Gee.HashSet(); Gee.HashSet appended_uids = new Gee.HashSet(); - Gee.HashSet discovered_uids = new Gee.HashSet(); + Gee.HashSet inserted_uids = new Gee.HashSet(); // Because the number of UIDs being processed can be immense in large folders, process // in a background thread @@ -251,12 +251,12 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde if (remote_uid.compare_to(local_latest_id.uid) > 0) appended_uids.add(remote_uid); else - discovered_uids.add(remote_uid); + inserted_uids.add(remote_uid); } }, cancellable); - debug("%s: changes since last seen: removed=%d appended=%d discovered=%d", to_string(), - removed_uids.size, appended_uids.size, discovered_uids.size); + debug("%s: changes since last seen: removed=%d appended=%d inserted=%d", to_string(), + removed_uids.size, appended_uids.size, inserted_uids.size); // fetch from the server the local store's required flags for all appended/inserted messages // (which is simply equal to all remaining remote UIDs) @@ -274,7 +274,8 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // store new messages and add IDs to the appended/discovered EmailIdentifier buckets Gee.Set appended_ids = new Gee.HashSet(); Gee.Set locally_appended_ids = new Gee.HashSet(); - Gee.Set discovered_ids = new Gee.HashSet(); + Gee.Set inserted_ids = new Gee.HashSet(); + Gee.Set locally_inserted_ids = new Gee.HashSet(); if (to_create != null && to_create.size > 0) { Gee.Map? created_or_merged = yield local_folder.create_or_merge_email_async( to_create, cancellable); @@ -294,8 +295,11 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde if (created) locally_appended_ids.add(id); - } else if (discovered_uids.contains(id.uid) && created) { - discovered_ids.add(id); + } else if (inserted_uids.contains(id.uid)) { + inserted_ids.add(id); + + if (created) + locally_inserted_ids.add(id); } } }, cancellable); @@ -303,7 +307,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde debug("%s: Finished creating/merging %d emails", to_string(), created_or_merged.size); } - check_open("normalize_folders (created/merged appended/discovered emails)"); + check_open("normalize_folders (created/merged appended/inserted emails)"); // Convert removed UIDs into EmailIdentifiers and detach immediately Gee.Set? removed_ids = null; @@ -327,6 +331,8 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // notify subscribers of changes // + Folder.CountChangeReason count_change_reason = Folder.CountChangeReason.NONE; + if (removed_ids != null && removed_ids.size > 0) { // there may be operations pending on the remote queue for these removed emails; notify // operations that the email has shuffled off this mortal coil @@ -335,20 +341,34 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // notify subscribers about emails that have been removed debug("%s: Notifying of %d removed emails since last opened", to_string(), removed_ids.size); notify_email_removed(removed_ids); + + count_change_reason |= Folder.CountChangeReason.REMOVED; } - // notify local discovered (i.e. emails that are in the interior of the vector not seen - // before -- this can happen during vector expansion when the app crashes or closes before - // writing out everything) - if (discovered_ids.size > 0) { - debug("%s: Notifying of %d discovered emails since last opened", to_string(), discovered_ids.size); - notify_email_discovered(discovered_ids); + // notify inserted (new email located somewhere inside the local vector) + if (inserted_ids.size > 0) { + debug("%s: Notifying of %d inserted emails since last opened", to_string(), inserted_ids.size); + notify_email_inserted(inserted_ids); + + count_change_reason |= Folder.CountChangeReason.INSERTED; + } + + // notify inserted (new email located somewhere inside the local vector that had to be + // created, i.e. no portion was stored locally) + if (locally_inserted_ids.size > 0) { + debug("%s: Notifying of %d locally inserted emails since last opened", to_string(), + locally_inserted_ids.size); + notify_email_locally_inserted(locally_inserted_ids); + + count_change_reason |= Folder.CountChangeReason.INSERTED; } // notify appended (new email added since the folder was last opened) if (appended_ids.size > 0) { debug("%s: Notifying of %d appended emails since last opened", to_string(), appended_ids.size); notify_email_appended(appended_ids); + + count_change_reason |= Folder.CountChangeReason.APPENDED; } // notify locally appended (new email never seen before added since the folder was last @@ -357,6 +377,14 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde debug("%s: Notifying of %d locally appended emails since last opened", to_string(), locally_appended_ids.size); notify_email_locally_appended(locally_appended_ids); + + count_change_reason |= Folder.CountChangeReason.APPENDED; + } + + if (count_change_reason != Folder.CountChangeReason.NONE) { + debug("%s: Notifying of %Xh count change reason (%d remote messages)", to_string(), + count_change_reason, remote_message_count); + notify_email_count_changed(remote_message_count, count_change_reason); } debug("%s: Completed normalize_folder", to_string()); @@ -730,10 +758,8 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde if (appended.size > 0) notify_email_appended(appended); - if (created.size > 0) { + if (created.size > 0) notify_email_locally_appended(created); - notify_email_discovered(created); - } notify_email_count_changed(remote_count, CountChangeReason.APPENDED); diff --git a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala index 0b7fb05b..6416bccf 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala @@ -171,8 +171,10 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen cb(null, null); // signal - if (created_ids.size > 0) - owner.notify_email_discovered(created_ids); + if (created_ids.size > 0) { + owner.notify_email_inserted(created_ids); + owner.notify_email_locally_inserted(created_ids); + } return ReplayOperation.Status.COMPLETED; } diff --git a/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala index 5fe1c230..e696c0dc 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala @@ -66,8 +66,8 @@ private class Geary.ImapEngine.ExpungeEmail : Geary.ImapEngine.SendReplayOperati public override async void backout_local_async() throws Error { yield engine.local_folder.mark_removed_async(removed_ids, false, cancellable); - engine.notify_email_appended(removed_ids); - engine.notify_email_count_changed(original_count, Geary.Folder.CountChangeReason.APPENDED); + engine.notify_email_inserted(removed_ids); + engine.notify_email_count_changed(original_count, Geary.Folder.CountChangeReason.INSERTED); } public override string describe_state() { diff --git a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala index cc081c1e..686e1030 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala @@ -103,8 +103,11 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation cancellable); // true means created - if (created_or_merged.get(email)) - engine.notify_email_discovered(new Collection.SingleItem(email.id)); + if (created_or_merged.get(email)) { + Gee.Collection ids = new Collection.SingleItem(email.id); + engine.notify_email_inserted(ids); + engine.notify_email_locally_inserted(ids); + } // if remote_email doesn't fulfill all required, pull from local database, which should now // be able to do all of that diff --git a/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala index 7f2ac225..30d3d258 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala @@ -66,8 +66,8 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation public override async void backout_local_async() throws Error { yield engine.local_folder.mark_removed_async(moved_ids, false, cancellable); - engine.notify_email_appended(moved_ids); - engine.notify_email_count_changed(original_count, Geary.Folder.CountChangeReason.APPENDED); + engine.notify_email_inserted(moved_ids); + engine.notify_email_count_changed(original_count, Geary.Folder.CountChangeReason.INSERTED); } public override string describe_state() {