[geary/mjog/invert-folder-class-hierarchy: 328/362] Geary.Folder, Geary.Account: Clean up interdependent signal handling




commit 0f3ce80d13a48ad918105384b1ebcd566ef80537
Author: Michael Gratton <mike vee net>
Date:   Sat Feb 13 23:25:51 2021 +1100

    Geary.Folder, Geary.Account: Clean up interdependent signal handling
    
    Ensure that signals in both classes can have default handlers. Ensure
    that for signals which should be emitted by both, that the default
    handler for the folder also emits the corresponding signal in the
    account. Ensure emitters of these signals are calling the correct ones.

 src/engine/api/geary-account.vala                  | 79 ++++++++++++++++------
 src/engine/api/geary-folder.vala                   | 60 +++++++++++-----
 src/engine/app/app-conversation-monitor.vala       | 16 ++---
 src/engine/app/app-search-folder.vala              | 10 ++-
 .../imap-engine-account-synchronizer.vala          | 10 ++-
 .../imap-engine/imap-engine-generic-account.vala   | 28 +++++---
 .../imap-engine/imap-engine-minimal-folder.vala    | 27 +++++---
 src/engine/outbox/outbox-folder.vala               |  1 -
 test/engine/app/app-conversation-monitor-test.vala | 13 ++--
 9 files changed, 165 insertions(+), 79 deletions(-)
---
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index 4a23f907a..fd4fa12b5 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -175,7 +175,7 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
      * ProblemReport} class provides context about the nature of the
      * problem itself.
      */
-    public signal void report_problem(Geary.ProblemReport problem);
+    public virtual signal void report_problem(Geary.ProblemReport problem);
 
     /**
      * Fired when folders become available or unavailable in the account.
@@ -191,9 +191,10 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
      *
      * @see sort_by_path
      */
-    public signal void
-        folders_available_unavailable(Gee.BidirSortedSet<Folder>? available,
-                                      Gee.BidirSortedSet<Folder>? unavailable);
+    public virtual signal void folders_available_unavailable(
+        Gee.BidirSortedSet<Folder>? available,
+        Gee.BidirSortedSet<Folder>? unavailable
+    );
 
     /**
      * Fired when new folders have been created.
@@ -208,7 +209,9 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
      * words, parents are listed before children, assuming the
      * collection is traversed in natural order.
      */
-    public signal void folders_created(Gee.BidirSortedSet<Geary.Folder> created);
+    public virtual signal void folders_created(
+        Gee.BidirSortedSet<Geary.Folder> created
+    );
 
     /**
      * Fired when existing folders are deleted.
@@ -223,38 +226,70 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
      * words, parents are listed before children, assuming the
      * collection is traversed in natural order.
      */
-    public signal void folders_deleted(Gee.BidirSortedSet<Geary.Folder> deleted);
+    public virtual signal void folders_deleted(
+        Gee.BidirSortedSet<Folder> deleted
+    );
 
     /**
      * Fired when a Folder's special use is detected having changed.
      */
-    public signal void folders_use_changed(Gee.Collection<Geary.Folder> altered);
+    public virtual signal void folders_use_changed(
+        Gee.Collection<Folder> altered
+    );
 
     /**
      * Emitted when emails are appended to a folder for this account.
+     *
+     * This is by default emitted by {@link Folder.email_appended}
+     * after that signal is emitted.
+     *
+     * @see email_inserted_into_folder
+     * @see Folder.email_appended
      */
-    public signal void email_appended_to_folder(Folder folder,
-                                                Gee.Collection<EmailIdentifier> ids);
+    public virtual signal void email_appended_to_folder(
+        Gee.Collection<EmailIdentifier> ids,
+        Folder source
+    );
 
     /**
      * Emitted when email messages are inserted into a folder for this account.
      *
+     * This is by default emitted by {@link Folder.email_inserted}
+     * after that signal is emitted.
+     *
+     * @see email_appended_to_folder
      * @see Folder.email_inserted
      */
-    public signal void email_inserted_into_folder(Folder folder,
-                                                  Gee.Collection<EmailIdentifier> ids);
+    public virtual signal void email_inserted_into_folder(
+        Gee.Collection<EmailIdentifier> ids,
+        Folder source
+    );
 
     /**
      * Emitted when email messages are removed from a folder for this account.
+     *
+     * This is by default emitted by {@link Folder.email_removed}
+     * after that signal is emitted.
+     *
+     * @see Folder.email_removed
      */
-    public signal void email_removed_from_folder(Folder folder,
-                                                 Gee.Collection<EmailIdentifier> ids);
+    public virtual signal void email_removed_from_folder(
+        Gee.Collection<EmailIdentifier> ids,
+        Folder source
+    );
 
     /**
      * Fired when the supplied email flags have changed from any folder.
+     *
+     * This is by default emitted by {@link
+     * Folder.email_flags_changed} after that signal is emitted.
+     *
+     * @see Folder.email_flags_changed
      */
-    public signal void email_flags_changed_in_folder(Folder folder,
-                                                     Gee.Map<EmailIdentifier,EmailFlags> map);
+    public virtual signal void email_flags_changed_in_folder(
+        Gee.Map<EmailIdentifier,EmailFlags> map,
+        Folder source
+    );
 
     /**
      * Emitted when email has been added to the account.
@@ -263,8 +298,10 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
      * added to an the account for the first time, including the
      * folder in they first appear.
      */
-    public signal void email_added(Gee.Collection<EmailIdentifier> ids,
-                                   Folder containing_folder);
+    public virtual signal void email_added(
+        Gee.Collection<EmailIdentifier> ids,
+        Folder containing_folder
+    );
 
     /**
      * Emitted when email has been removed from the account.
@@ -274,12 +311,16 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
      * necessarily immediately) after they have been removed from all
      * folders they have been present.
      */
-    public signal void email_removed(Gee.Collection<EmailIdentifier> ids);
+    public virtual signal void email_removed(
+        Gee.Collection<EmailIdentifier> ids
+    );
 
     /**
      * Emitted when email mesages have been fully downloaded.
      */
-    public signal void email_complete(Gee.Collection<EmailIdentifier> ids);
+    public virtual signal void email_complete(
+        Gee.Collection<EmailIdentifier> ids
+    );
 
 
     protected Account(AccountInformation information,
diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala
index 6e2556929..a070264dd 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -580,10 +580,16 @@ public interface Geary.Folder : GLib.Object, Logging.Source {
      * the "top" of the vector of messages, for example, newly
      * delivered email.
      *
+     * This will be default also emit {@link
+     * Account.email_appended_to_folder}.
+     *
      * @see email_inserted
+     * @see Account.email_appended_to_folder
      */
-    public virtual signal void email_appended(Gee.Collection<EmailIdentifier> ids) {
-        this.account.email_appended_to_folder(this, ids);
+    public virtual signal void email_appended(
+        Gee.Collection<EmailIdentifier> ids
+    ) {
+        this.account.email_appended_to_folder(ids, this);
     }
 
     /**
@@ -596,33 +602,49 @@ public interface Geary.Folder : GLib.Object, Logging.Source {
      * including vector expansion, but note that newly received
      * messages are appended and notified via {@link email_appended}.
      *
+     * This will be default also emit {@link
+     * Account.email_inserted_into_folder}.
+     *
      * @see email_appended
+     * @see Account.email_inserted_into_folder
      */
-    public virtual signal void email_inserted(Gee.Collection<EmailIdentifier> ids) {
-        this.account.email_inserted_into_folder(this, ids);
+    public virtual signal void email_inserted(
+        Gee.Collection<EmailIdentifier> ids
+    ) {
+        this.account.email_inserted_into_folder(ids, this);
     }
 
     /**
      * Fired when email has been removed (deleted or moved) from the folder.
      *
-     * This may occur due to the local user's action or reported from the server (i.e. another
-     * client has performed the action).  Email positions greater than the removed emails are
-     * affected.
+     * This may occur due to the local user's action or reported from
+     * the server (i.e. another client has performed the action).
+     * Email positions greater than the removed emails are affected.
+     *
+     * This will be default also emit {@link
+     * Account.email_removed_from_folder}.
      *
-     * ''Note:'' It's possible for the remote server to report a message has been removed that is not
-     * known locally (and therefore the caller could not have record of).  If this happens, this
-     * signal will ''not'' fire, although {@link email_count_changed} will.
+     * @see Account.email_removed_from_folder
      */
-    public virtual signal void email_removed(Gee.Collection<EmailIdentifier> ids) {
-        this.account.email_removed_from_folder(this, ids);
+    public virtual signal void email_removed(
+        Gee.Collection<EmailIdentifier> ids
+    ) {
+        this.account.email_removed_from_folder(ids, this);
     }
 
     /**
      * Fired when the supplied email flags have changed, whether due to local action or reported by
      * the server.
+     *
+     * This will be default also emit {@link
+     * Account.email_flags_changed_in_folder}.
+     *
+     * @see Account.email_flags_changed_in_folder
      */
-    public virtual signal void email_flags_changed(Gee.Map<EmailIdentifier,EmailFlags> map) {
-        this.account.email_flags_changed_in_folder(this, map);
+    public virtual signal void email_flags_changed(
+        Gee.Map<EmailIdentifier,EmailFlags> map
+    ) {
+        this.account.email_flags_changed_in_folder(map, this);
     }
 
     /**
@@ -632,15 +654,21 @@ public interface Geary.Folder : GLib.Object, Logging.Source {
      * and {@link email_removed} (although see the note at
      * email_removed).
      */
-    public virtual signal void email_count_changed(int new_count, CountChangeReason reason);
+    public virtual signal void email_count_changed(
+        int new_count, CountChangeReason reason
+    );
 
     /**
      * Fired when the folder's special use has changed.
      *
      * This will usually happen when the local object has been updated
      * with data discovered from the remote account.
+     *
+     * @see Account.folders_use_changed
      */
-    public virtual signal void use_changed(SpecialUse old_use, SpecialUse new_use);
+    public virtual signal void use_changed(
+        SpecialUse old_use, SpecialUse new_use
+    );
 
 
     /**
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index b3385d92d..dc54f1e38 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -771,15 +771,15 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         this.queue.add(new RemoveOperation(this, this.base_folder, removed));
     }
 
-    private void on_account_email_appended(Folder folder,
-                                           Gee.Collection<EmailIdentifier> added) {
+    private void on_account_email_appended(Gee.Collection<EmailIdentifier> added,
+                                           Folder folder) {
         if (folder != this.base_folder) {
             this.queue.add(new ExternalAppendOperation(this, folder, added));
         }
     }
 
-    private void on_account_email_inserted(Folder folder,
-                                           Gee.Collection<EmailIdentifier> inserted) {
+    private void on_account_email_inserted(Gee.Collection<EmailIdentifier> inserted,
+                                           Folder folder) {
         // ExternalAppendOperation will check to determine if the
         // email is relevant for some existing conversation before
         // adding it, which is what we want here.
@@ -788,15 +788,15 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         }
     }
 
-    private void on_account_email_removed(Folder folder,
-                                          Gee.Collection<EmailIdentifier> removed) {
+    private void on_account_email_removed(Gee.Collection<EmailIdentifier> removed,
+                                          Folder folder) {
         if (folder != this.base_folder) {
             this.queue.add(new RemoveOperation(this, folder, removed));
         }
     }
 
-    private void on_account_email_flags_changed(Geary.Folder folder,
-                                                Gee.Map<EmailIdentifier,EmailFlags> map) {
+    private void on_account_email_flags_changed(Gee.Map<EmailIdentifier,EmailFlags> map,
+                                                Geary.Folder folder) {
         Gee.HashSet<EmailIdentifier> inserted_ids = new Gee.HashSet<EmailIdentifier>();
         Gee.HashSet<EmailIdentifier> removed_ids = new Gee.HashSet<EmailIdentifier>();
         Gee.HashSet<Conversation> removed_conversations = new Gee.HashSet<Conversation>();
diff --git a/src/engine/app/app-search-folder.vala b/src/engine/app/app-search-folder.vala
index 19496aa52..4e9b65dfa 100644
--- a/src/engine/app/app-search-folder.vala
+++ b/src/engine/app/app-search-folder.vala
@@ -608,7 +608,6 @@ public class Geary.App.SearchFolder : BaseObject,
             Folder.CountChangeReason reason = CountChangeReason.NONE;
             if (removed.size > 0) {
                 email_removed(removed);
-                this.account.email_removed_from_folder(this, removed);
                 reason |= Folder.CountChangeReason.REMOVED;
             }
             if (added.size > 0) {
@@ -618,7 +617,6 @@ public class Geary.App.SearchFolder : BaseObject,
                 // ConversationMonitor's inability to handle that
                 // gracefully (#7464), we always use INSERTED for now.
                 email_inserted(added);
-                this.account.email_inserted_into_folder(this, removed);
                 reason |= Folder.CountChangeReason.INSERTED;
             }
             if (reason != CountChangeReason.NONE) {
@@ -672,15 +670,15 @@ public class Geary.App.SearchFolder : BaseObject,
         }
     }
 
-    private void on_email_added(Geary.Folder source,
-                                Gee.Collection<EmailIdentifier> ids) {
+    private void on_email_added(Gee.Collection<EmailIdentifier> ids,
+                                Geary.Folder source) {
         if (this.query != null) {
             this.append.begin(source, ids);
         }
     }
 
-    private void on_email_removed(Geary.Folder source,
-                                  Gee.Collection<EmailIdentifier> ids) {
+    private void on_email_removed(Gee.Collection<EmailIdentifier> ids,
+                                  Geary.Folder source) {
         if (this.query != null) {
             this.remove.begin(source, ids);
         }
diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala 
b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
index f57b8a634..d0861546e 100644
--- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
+++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
@@ -266,10 +266,9 @@ private class Geary.ImapEngine.FullFolderSync : RefreshFolderSync {
                 yield local_folder.detach_emails_before_timestamp(max_epoch,
                                                                   cancellable);
             if (detached_ids != null) {
+                // Only notify for the folder here, GC should handle
+                // notifying when deleted account-wide
                 this.folder.email_removed(detached_ids);
-                this.folder.account.email_removed_from_folder(
-                    this.folder, detached_ids
-                );
 
                 // Ensure a foreground GC is queued so any email now
                 // folderless will be reaped
@@ -441,10 +440,9 @@ private class Geary.ImapEngine.TruncateToEpochFolderSync : FolderSync {
                 yield local_folder.detach_emails_before_timestamp(max_epoch,
                                                                   cancellable);
             if (detached_ids != null) {
+                // Only notify for the folder here, GC should handle
+                // notifying when deleted account-wide
                 this.folder.email_removed(detached_ids);
-                this.folder.account.email_removed_from_folder(
-                    this.folder, detached_ids
-                );
                 this.post_idle_detach_op.messages_detached();
             }
         }
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index a34d5c2e8..80aed7181 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -149,10 +149,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
         this.last_storage_cleanup = yield this.local.fetch_last_cleanup_async(cancellable);
         this.notify["last_storage_cleanup"].connect(on_last_storage_cleanup_notify);
-        this.email_appended_to_folder.connect(on_folder_contents_altered);
-        this.email_inserted_into_folder.connect(on_folder_contents_altered);
-        this.email_removed_from_folder.connect(on_folder_contents_altered);
-        this.email_flags_changed_in_folder.connect(on_folder_contents_altered);
+        this.email_appended_to_folder.connect(on_folder_email_contents_altered);
+        this.email_inserted_into_folder.connect(on_folder_email_contents_altered);
+        this.email_removed_from_folder.connect(on_folder_email_contents_altered);
+        this.email_flags_changed_in_folder.connect(on_folder_email_flags_altered);
 
         this.open = true;
         opened();
@@ -183,10 +183,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
             debug("Error stopping SMTP service: %s", err.message);
         }
 
-        this.email_appended_to_folder.disconnect(on_folder_contents_altered);
-        this.email_inserted_into_folder.disconnect(on_folder_contents_altered);
-        this.email_removed_from_folder.disconnect(on_folder_contents_altered);
-        this.email_flags_changed_in_folder.disconnect(on_folder_contents_altered);
+        this.email_appended_to_folder.disconnect(on_folder_email_contents_altered);
+        this.email_inserted_into_folder.disconnect(on_folder_email_contents_altered);
+        this.email_removed_from_folder.disconnect(on_folder_email_contents_altered);
+        this.email_flags_changed_in_folder.disconnect(on_folder_email_flags_altered);
 
         // Halt internal tasks early so they stop using local and
         // remote connections.
@@ -1080,7 +1080,17 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         }
     }
 
-    private void on_folder_contents_altered(Folder folder) {
+    private void on_folder_email_contents_altered(
+        Gee.Collection<EmailIdentifier> ids,
+        Folder folder
+    ) {
+        schedule_unseen_update(folder);
+    }
+
+    private void on_folder_email_flags_altered(
+        Gee.Map<EmailIdentifier,EmailFlags> map,
+        Folder folder
+    ) {
         schedule_unseen_update(folder);
     }
 
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 303dcd4a5..61bc7a72a 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -164,15 +164,7 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
             }
             set_use(NONE);
         }
-    }
-
-    public void set_use(Folder.SpecialUse new_use) {
-        var old_use = this._used_as;
-        this._used_as = new_use;
-        if (old_use != new_use) {
-            use_changed(old_use, new_use);
-            update_harvester();
-        }
+        this.account.folders_use_changed(Collection.single(this));
     }
 
     /** {@inheritDoc} */
@@ -278,6 +270,23 @@ private class Geary.ImapEngine.MinimalFolder : BaseObject,
         return this.remote_session;
     }
 
+    /**
+     * Sets the special use for this folder.
+     *
+     * Note this emits the {@link Folder.use_changed} signal as
+     * required, but does not emit {@link
+     * Account.folders_use_changed}. Callers will need arrange to see
+     * that emitted.
+     */
+    internal void set_use(Folder.SpecialUse new_use) {
+        var old_use = this._used_as;
+        if (old_use != new_use) {
+            this._used_as = new_use;
+            use_changed(old_use, new_use);
+            update_harvester();
+        }
+    }
+
     private async void check_remote_session() {
         bool can_be_connected = (
             this._account.imap.current_status == CONNECTED
diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala
index f82c6ac32..ce3348522 100644
--- a/src/engine/outbox/outbox-folder.vala
+++ b/src/engine/outbox/outbox-folder.vala
@@ -188,7 +188,6 @@ public class Geary.Outbox.Folder : BaseObject,
 
             email_removed(removed);
             email_count_changed(final_count, REMOVED);
-            this.account.email_removed(removed);
         }
     }
 
diff --git a/test/engine/app/app-conversation-monitor-test.vala 
b/test/engine/app/app-conversation-monitor-test.vala
index c54ecd47e..bd8c8911d 100644
--- a/test/engine/app/app-conversation-monitor-test.vala
+++ b/test/engine/app/app-conversation-monitor-test.vala
@@ -395,14 +395,14 @@ class Geary.App.ConversationMonitorTest : TestCase {
 
         // Should not be added, since it's actually in the base folder
         this.account.email_appended_to_folder(
-            this.base_folder,
-            new Gee.ArrayList<EmailIdentifier>.wrap({e2.id})
+            new Gee.ArrayList<EmailIdentifier>.wrap({e2.id}),
+            this.base_folder
         );
 
         // Should be added, since it's an external message
         this.account.email_appended_to_folder(
-            this.other_folder,
-            new Gee.ArrayList<EmailIdentifier>.wrap({e3.id})
+            new Gee.ArrayList<EmailIdentifier>.wrap({e3.id}),
+            this.other_folder
         );
 
         wait_for_signal(monitor, "conversations-added");
@@ -432,7 +432,10 @@ class Geary.App.ConversationMonitorTest : TestCase {
         Gee.HashMap<EmailIdentifier,EmailFlags> flags_changed =
             new Gee.HashMap<EmailIdentifier,EmailFlags>();
         flags_changed.set(e1.id, new EmailFlags.with(EmailFlags.DELETED));
-        this.account.email_flags_changed_in_folder(this.base_folder, flags_changed);
+        this.account.email_flags_changed_in_folder(
+            flags_changed,
+            this.base_folder
+        );
 
         this.base_folder.expect_call("list_email_by_sparse_id_async");
         this.base_folder.expect_call("list_email_by_id_async");


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]