[geary/wip/778276-better-flag-updates] Create and use a common FolderOperation account operation class.

[Michael Gratton <mjog src gnome org>]

commit 9e93062d6f3c19b7d827c3f85105a7d64a74b050
Author: Michael James Gratton <mike vee net>
Date:   Tue Dec 19 10:49:05 2017 +1100

    Create and use a common FolderOperation account operation class.

 .../imap-engine/imap-engine-account-operation.vala |   73 +++++++++++++++++++-
 .../imap-engine/imap-engine-generic-account.vala   |   51 ++++++--------
 .../engine/imap-engine/account-processor-test.vala |   35 +++++++--
 3 files changed, 120 insertions(+), 39 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-account-operation.vala 
b/src/engine/imap-engine/imap-engine-account-operation.vala
index 3913021..ffab9ca 100644
--- a/src/engine/imap-engine/imap-engine-account-operation.vala
+++ b/src/engine/imap-engine/imap-engine-account-operation.vala
@@ -15,12 +15,27 @@
  *
  * Execution of the operation is managed by {@link
  * AccountProcessor}. Since the processor will not en-queue duplicate
- * operations, implementations may override the {@link equal_to}
- * method to ensure that the same operation is not queued twice.
+ * operations, implementations of this class may override the {@link
+ * equal_to} method to specify which instances are considered to be
+ * duplicates.
  */
 public abstract class Geary.ImapEngine.AccountOperation : Geary.BaseObject {
 
 
+    /** The account this operation applies to. */
+    protected weak Geary.Account account;
+
+
+    /**
+     * Constructs a new account operation.
+     *
+     * The passed in `account` will be the account the operation will
+     * apply to.
+     */
+    protected AccountOperation(Geary.Account account) {
+        this.account = account;
+    }
+
     /**
      * Fired by after processing when the operation has completed.
      *
@@ -84,3 +99,57 @@ public abstract class Geary.ImapEngine.AccountOperation : Geary.BaseObject {
     }
 
 }
+
+
+/**
+ * An account operation that applies to a specific folder.
+ *
+ * By default, instances of this class require that another operation
+ * applies to the same folder as well as having the same type to be
+ * considered equal, for the purpose of not en-queuing duplicate
+ * operations.
+ */
+public abstract class Geary.ImapEngine.FolderOperation : AccountOperation {
+
+
+    /** The folder this operation applies to. */
+    protected weak Geary.Folder folder;
+
+
+    /**
+     * Constructs a new folder operation.
+     *
+     * The passed in `folder` and `account` will be the objects the
+     * operation will apply to.
+     */
+    protected FolderOperation(Geary.Account account, Geary.Folder folder) {
+        base(account);
+        this.folder = folder;
+    }
+
+    /**
+     * Determines if another operation is equal to this.
+     *
+     * This method compares both chain's up to {@link
+     * AccountOperation.equal_to} and if equal, compares the paths of
+     * both operation's folders to determine if `op` is equal to this
+     * operation.
+     */
+    public override bool equal_to(AccountOperation op) {
+        return (
+            base.equal_to(op) &&
+            this.folder.path.equal_to(((FolderOperation) op).folder.path)
+        );
+    }
+
+    /**
+     * Provides a representation of this operation for debugging.
+     *
+     * The return value will include its folder's path and the name of
+     * the class.
+     */
+    public override string to_string() {
+        return "%s(%s)".printf(base.to_string(), folder.path.to_string());
+    }
+
+}
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index a6a7b71..ccac9d5 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -811,7 +811,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 internal class Geary.ImapEngine.LoadFolders : AccountOperation {
 
 
-    private weak GenericAccount account;
     private weak ImapDB.Account local;
     private Geary.SpecialFolderType[] specials;
 
@@ -819,16 +818,17 @@ internal class Geary.ImapEngine.LoadFolders : AccountOperation {
     internal LoadFolders(GenericAccount account,
                          ImapDB.Account local,
                          Geary.SpecialFolderType[] specials) {
-        this.account = account;
+        base(account);
         this.local = local;
         this.specials = specials;
     }
 
     public override async void execute(Cancellable cancellable) throws Error {
+        GenericAccount generic = (GenericAccount) this.account;
         Gee.List<ImapDB.Folder> folders = new Gee.LinkedList<ImapDB.Folder>();
         yield enumerate_local_folders_async(folders, null, cancellable);
         debug("%s: found %u folders", to_string(), folders.size);
-        this.account.add_folders(folders, true);
+        generic.add_folders(folders, true);
 
         if (!folders.is_empty) {
             // If we have some folders to load, then this isn't the
@@ -836,9 +836,12 @@ internal class Geary.ImapEngine.LoadFolders : AccountOperation {
             // exist
             foreach (Geary.SpecialFolderType special in this.specials) {
                 try {
-                    yield this.account.ensure_special_folder_async(special, cancellable);
+                    yield generic.ensure_special_folder_async(special, cancellable);
                 } catch (Error e) {
-                    warning("Unable to ensure special folder %s: %s", special.to_string(), e.message);
+                    warning(
+                        "Unable to ensure special folder %s: %s",
+                        special.to_string(), e.message
+                    );
                 }
             }
         }
@@ -876,7 +879,7 @@ internal class Geary.ImapEngine.LoadFolders : AccountOperation {
 internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
 
 
-    private weak GenericAccount account;
+    private weak GenericAccount generic_account;
     private weak Imap.Account remote;
     private weak ImapDB.Account local;
     private Gee.Collection<FolderPath> local_folders;
@@ -888,7 +891,8 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
                                  ImapDB.Account local,
                                  Gee.Collection<FolderPath> local_folders,
                                  Geary.SpecialFolderType[] specials) {
-        this.account = account;
+        base(account);
+        this.generic_account = account;
         this.remote = remote;
         this.local = local;
         this.local_folders = local_folders;
@@ -1012,12 +1016,13 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
                 debug("Unable to fetch local folder after cloning: %s", convert_err.message);
             }
         }
-        this.account.add_folders(to_build, false);
+        this.generic_account.add_folders(to_build, false);
 
         if (remote_folders_suspect) {
             debug("Skipping removing folders due to prior errors");
         } else {
-            Gee.List<MinimalFolder> removed = this.account.remove_folders(to_remove);
+            Gee.List<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.get_path_length() - a.path.get_path_length());
@@ -1046,13 +1051,15 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
                 else
                     debug("Unable to report %s altered: no local representation", altered_path.to_string());
             }
-            this.account.update_folders(altered);
+            this.generic_account.update_folders(altered);
         }
 
         // Ensure each of the important special folders we need already exist
         foreach (Geary.SpecialFolderType special in this.specials) {
             try {
-                yield this.account.ensure_special_folder_async(special, cancellable);
+                yield this.generic_account.ensure_special_folder_async(
+                    special, cancellable
+                );
             } catch (Error e) {
                 warning("Unable to ensure special folder %s: %s", special.to_string(), e.message);
             }
@@ -1067,11 +1074,9 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
  * This performs a IMAP STATUS on the folder, but only if it is not
  * open - if it is open it is already maintaining its unseen count.
  */
-internal class Geary.ImapEngine.RefreshFolderUnseen : AccountOperation {
+internal class Geary.ImapEngine.RefreshFolderUnseen : FolderOperation {
 
 
-    private weak MinimalFolder folder;
-    private weak GenericAccount account;
     private weak Imap.Account remote;
     private weak ImapDB.Account local;
 
@@ -1080,23 +1085,11 @@ internal class Geary.ImapEngine.RefreshFolderUnseen : AccountOperation {
                                  GenericAccount account,
                                  Imap.Account remote,
                                  ImapDB.Account local) {
-        this.folder = folder;
-        this.account = account;
+        base(account, folder);
         this.remote = remote;
         this.local = local;
     }
 
-    public override bool equal_to(AccountOperation op) {
-        return (
-            base.equal_to(op) &&
-            this.folder.path.equal_to(((RefreshFolderUnseen) op).folder.path)
-        );
-    }
-
-    public override string to_string() {
-        return "%s(%s)".printf(base.to_string(), folder.path.to_string());
-    }
-
     public override async void execute(Cancellable cancellable) throws Error {
         if (this.folder.get_open_state() == Geary.Folder.OpenState.CLOSED) {
             Imap.Folder remote_folder = yield remote.fetch_folder_cached_async(
@@ -1106,13 +1099,13 @@ internal class Geary.ImapEngine.RefreshFolderUnseen : AccountOperation {
             );
 
             if (remote_folder.properties.have_contents_changed(
-                    this.folder.local_folder.get_properties(),
+                    ((MinimalFolder) this.folder).local_folder.get_properties(),
                     this.folder.to_string())) {
                 yield local.update_folder_status_async(
                     remote_folder, false, true, cancellable
                 );
 
-                this.account.update_folder(this.folder);
+                ((GenericAccount) this.account).update_folder(this.folder);
             }
         }
     }
diff --git a/test/engine/imap-engine/account-processor-test.vala 
b/test/engine/imap-engine/account-processor-test.vala
index 7c14219..15a112f 100644
--- a/test/engine/imap-engine/account-processor-test.vala
+++ b/test/engine/imap-engine/account-processor-test.vala
@@ -20,6 +20,10 @@ public class Geary.ImapEngine.AccountProcessorTest : Gee.TestCase {
 
         private Nonblocking.Spinlock spinlock = new Nonblocking.Spinlock();
 
+        internal TestOperation(Geary.Account account) {
+            base(account);
+        }
+
         public override async void execute(Cancellable cancellable)
             throws Error {
             print("Test op/");
@@ -37,10 +41,16 @@ public class Geary.ImapEngine.AccountProcessorTest : Gee.TestCase {
 
     public class OtherOperation : TestOperation {
 
+        internal OtherOperation(Geary.Account account) {
+            base(account);
+        }
+
     }
 
 
-    private AccountProcessor processor;
+    private AccountProcessor? processor = null;
+    private Geary.Account? account = null;
+    private Geary.AccountInformation? info = null;
     private uint succeeded;
     private uint failed;
     private uint completed;
@@ -53,17 +63,26 @@ public class Geary.ImapEngine.AccountProcessorTest : Gee.TestCase {
         add_test("test_duplicate", test_duplicate);
         add_test("test_stop", test_stop);
 
+        // XXX this has to be here instead of in set_up for some
+        // reason...
         this.processor = new AccountProcessor("processor");
     }
 
     public override void set_up() {
+        this.info = new Geary.AccountInformation(
+            "test-info",
+            File.new_for_path("."),
+            File.new_for_path(".")
+        );
+        this.account = new Geary.MockAccount("test-account", this.info);
+
         this.succeeded = 0;
         this.failed = 0;
         this.completed = 0;
     }
 
     public void test_success() {
-        TestOperation op = setup_operation(new TestOperation());
+        TestOperation op = setup_operation(new TestOperation(this.account));
 
         this.processor.enqueue(op);
         assert(this.processor.waiting == 1);
@@ -77,7 +96,7 @@ public class Geary.ImapEngine.AccountProcessorTest : Gee.TestCase {
     }
 
     public void test_failure() {
-        TestOperation op = setup_operation(new TestOperation());
+        TestOperation op = setup_operation(new TestOperation(this.account));
         op.throw_error = true;
 
         AccountOperation? error_op = null;
@@ -98,9 +117,9 @@ public class Geary.ImapEngine.AccountProcessorTest : Gee.TestCase {
     }
 
     public void test_duplicate() {
-        TestOperation op1 = setup_operation(new TestOperation());
-        TestOperation op2 = setup_operation(new TestOperation());
-        TestOperation op3 = setup_operation(new OtherOperation());
+        TestOperation op1 = setup_operation(new TestOperation(this.account));
+        TestOperation op2 = setup_operation(new TestOperation(this.account));
+        TestOperation op3 = setup_operation(new OtherOperation(this.account));
 
         this.processor.enqueue(op1);
         this.processor.enqueue(op2);
@@ -111,9 +130,9 @@ public class Geary.ImapEngine.AccountProcessorTest : Gee.TestCase {
     }
 
     public void test_stop() {
-        TestOperation op1 = setup_operation(new TestOperation());
+        TestOperation op1 = setup_operation(new TestOperation(this.account));
         op1.wait_for_cancel = true;
-        TestOperation op2 = setup_operation(new OtherOperation());
+        TestOperation op2 = setup_operation(new OtherOperation(this.account));
 
         this.processor.enqueue(op1);
         this.processor.enqueue(op2);