[geary/mjog/replay-queue-fixes: 1/2] Geary.ImapEngine: Ensure actual replay ops interleave correctly

[Michael Gratton <mjog src gnome org>]

commit 224dbf08d33a7b3590623a9d1141cb0d3f4b23df
Author: Michael Gratton <mike vee net>
Date:   Tue Feb 23 22:55:20 2021 +1100

    Geary.ImapEngine: Ensure actual replay ops interleave correctly
    
    Ensure `ReplayAppend`, `ReplayUpdate` and `ReplayRemoval` ops all
    run as remote-only ops, so that they can't be incorrectly interleaved.
    For example an update queued after a remove can't be executed first
    in the local queue.

 .../imap-engine/replay-ops/imap-engine-replay-append.vala   |  7 +++++--
 .../imap-engine/replay-ops/imap-engine-replay-removal.vala  | 13 +++++--------
 .../imap-engine/replay-ops/imap-engine-replay-update.vala   | 11 ++++++-----
 3 files changed, 16 insertions(+), 15 deletions(-)
---
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala 
b/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala
index 2ed094388..720ca0c0e 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala
@@ -20,8 +20,11 @@ private class Geary.ImapEngine.ReplayAppend : Geary.ImapEngine.ReplayOperation {
                         int remote_count,
                         Gee.List<Imap.SequenceNumber> positions,
                         Cancellable? cancellable) {
-        // IGNORE remote errors because the reconnect will re-normalize the folder, making this
-        // append moot
+        // Since this is a report op, both ReplayRemove and
+        // ReplayUpdate must also be run as remote ops so their
+        // effects are interleaved correctly. IGNORE remote errors
+        // because the reconnect will re-normalize the folder, making
+        // this append moot
         base ("Append", Scope.REMOTE_ONLY, OnError.IGNORE_REMOTE);
 
         this.owner = owner;
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala 
b/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
index 6b8a0b8a3..60f186e1b 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
@@ -16,8 +16,11 @@ private class Geary.ImapEngine.ReplayRemoval : Geary.ImapEngine.ReplayOperation
 
 
     public ReplayRemoval(MinimalFolder owner, int remote_count, Imap.SequenceNumber position) {
-        // remote error will cause folder to reconnect and re-normalize, making this remove moot
-        base ("Removal", Scope.LOCAL_AND_REMOTE, OnError.IGNORE_REMOTE);
+        // Although technically a local-only operation, must treat as
+        // remote to ensure it's processed in-order with ReplayAppend
+        // and ReplayUpdate operations. Remote error will cause folder
+        // to reconnect and re-normalize, making this remove moot
+        base ("Removal", Scope.REMOTE_ONLY, OnError.IGNORE_REMOTE);
 
         this.owner = owner;
         this.remote_count = remote_count;
@@ -37,12 +40,6 @@ private class Geary.ImapEngine.ReplayRemoval : Geary.ImapEngine.ReplayOperation
         // this ReplayOperation doesn't do remote removes, it reacts to them
     }
 
-    public override async ReplayOperation.Status replay_local_async() throws Error {
-        // Although technically a local-only operation, must treat as remote to ensure it's
-        // processed in-order with ReplayAppend operations
-        return ReplayOperation.Status.CONTINUE;
-    }
-
     public override async void replay_remote_async(Imap.FolderSession remote)
         throws GLib.Error {
         debug("%s: ReplayRemoval this.position=%s reported_remote_count=%d",
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-replay-update.vala 
b/src/engine/imap-engine/replay-ops/imap-engine-replay-update.vala
index eb0bc3e7f..beb573553 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-replay-update.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-replay-update.vala
@@ -22,7 +22,10 @@ private class Geary.ImapEngine.ReplayUpdate : Geary.ImapEngine.ReplayOperation {
                         int remote_count,
                         Imap.SequenceNumber position,
                         Imap.FetchedData data) {
-        base ("Update", Scope.LOCAL_ONLY, OnError.RETRY);
+        // Although technically a local-only operation, must treat as
+        // remote to ensure it's processed in-order with ReplayAppend
+        // and ReplayRemove operations
+        base ("Update", Scope.REMOTE_ONLY, OnError.RETRY);
 
         this.owner = owner;
         this.remote_count = remote_count;
@@ -30,8 +33,8 @@ private class Geary.ImapEngine.ReplayUpdate : Geary.ImapEngine.ReplayOperation {
         this.data = data;
     }
 
-    public override async ReplayOperation.Status replay_local_async()
-        throws Error {
+    public override async void replay_remote_async(Imap.FolderSession remote)
+        throws GLib.Error {
         Imap.MessageFlags? message_flags =
             this.data.data_map.get(Imap.FetchDataSpecifier.FLAGS) as Imap.MessageFlags;
         if (message_flags != null) {
@@ -66,8 +69,6 @@ private class Geary.ImapEngine.ReplayUpdate : Geary.ImapEngine.ReplayOperation {
             debug("%s Don't know what to do without any FLAGS: %s",
                   to_string(), this.data.to_string());
         }
-
-        return ReplayOperation.Status.COMPLETED;
     }
 
     public override string describe_state() {