[geary/wip/721828-undo-2: 2/2] End-to-end functionality, although not as snappy as we would like



commit 67fae4737f17f02b0ba8d78c8c30db1b9702010d
Author: Jim Nelson <jim yorba org>
Date:   Tue Jan 13 16:08:43 2015 -0800

    End-to-end functionality, although not as snappy as we would like

 src/CMakeLists.txt                                 |    1 +
 src/client/application/geary-controller.vala       |   30 ++++--
 src/engine/api/geary-folder-supports-archive.vala  |   10 +-
 src/engine/api/geary-folder-supports-move.vala     |    4 +-
 .../gmail/imap-engine-gmail-folder.vala            |   14 ++-
 .../imap-engine/imap-engine-generic-account.vala   |    3 +
 .../imap-engine/imap-engine-minimal-folder.vala    |   11 ++-
 .../imap-engine/imap-engine-revokable-move.vala    |  123 ++++++++++++++++++++
 .../replay-ops/imap-engine-move-email.vala         |   11 ++-
 9 files changed, 190 insertions(+), 17 deletions(-)
---
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b8375dd..5d95e6b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -192,6 +192,7 @@ engine/imap-engine/imap-engine-generic-folder.vala
 engine/imap-engine/imap-engine-minimal-folder.vala
 engine/imap-engine/imap-engine-replay-operation.vala
 engine/imap-engine/imap-engine-replay-queue.vala
+engine/imap-engine/imap-engine-revokable-move.vala
 engine/imap-engine/imap-engine-send-replay-operation.vala
 engine/imap-engine/gmail/imap-engine-gmail-account.vala
 engine/imap-engine/gmail/imap-engine-gmail-all-mail-folder.vala
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 9622aea..afac5f4 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1870,10 +1870,19 @@ public class GearyController : Geary.BaseObject {
             return;
         
         Geary.FolderSupport.Move? supports_move = current_folder as Geary.FolderSupport.Move;
-        if (supports_move == null)
-            return;
-        
-        supports_move.move_email_async.begin(ids, destination.path, cancellable_folder);
+        if (supports_move != null)
+            move_conversation_async.begin(supports_move, ids, destination.path, cancellable_folder);
+    }
+    
+    private async void move_conversation_async(Geary.FolderSupport.Move source_folder,
+        Gee.List<Geary.EmailIdentifier> ids, Geary.FolderPath destination, Cancellable? cancellable) {
+        try {
+            save_revokable(yield source_folder.move_email_async(ids, destination, cancellable),
+                _("Undo move"));
+        } catch (Error err) {
+            debug("%s: Unable to move %d emails: %s", source_folder.to_string(), ids.size,
+                err.message);
+        }
     }
     
     private void on_open_attachment(Geary.Attachment attachment) {
@@ -2395,10 +2404,13 @@ public class GearyController : Geary.BaseObject {
             debug("Archiving selected messages");
             
             Geary.FolderSupport.Archive? supports_archive = current_folder as Geary.FolderSupport.Archive;
-            if (supports_archive == null)
+            if (supports_archive == null) {
                 debug("Folder %s doesn't support archive", current_folder.to_string());
-            else
-                yield supports_archive.archive_email_async(ids, cancellable);
+            } else {
+                save_revokable(yield supports_archive.archive_email_async(ids, cancellable),
+                    _("Undo archive"));
+            }
+            
             return;
         }
         
@@ -2410,7 +2422,9 @@ public class GearyController : Geary.BaseObject {
                     Geary.SpecialFolderType.TRASH, cancellable)).path;
                 Geary.FolderSupport.Move? supports_move = current_folder as Geary.FolderSupport.Move;
                 if (supports_move != null) {
-                    yield supports_move.move_email_async(ids, trash_path, cancellable);
+                    save_revokable(yield supports_move.move_email_async(ids, trash_path, cancellable),
+                        _("Undo trash"));
+                    
                     return;
                 }
             }
diff --git a/src/engine/api/geary-folder-supports-archive.vala 
b/src/engine/api/geary-folder-supports-archive.vala
index 9796a76..73ce06d 100644
--- a/src/engine/api/geary-folder-supports-archive.vala
+++ b/src/engine/api/geary-folder-supports-archive.vala
@@ -18,21 +18,25 @@ public interface Geary.FolderSupport.Archive : Geary.Folder {
      * Archives the specified emails from the folder.
      *
      * The { link Geary.Folder} must be opened prior to attempting this operation.
+     *
+     * @returns A { link Geary.Revokable} that may be used to revoke (undo) this operation later.
      */
-    public abstract async void archive_email_async(Gee.List<Geary.EmailIdentifier> email_ids,
+    public abstract async Geary.Revokable? archive_email_async(Gee.List<Geary.EmailIdentifier> email_ids,
         Cancellable? cancellable = null) throws Error;
     
     /**
      * Archive one email from the folder.
      *
      * The { link Geary.Folder} must be opened prior to attempting this operation.
+     *
+     * @returns A { link Geary.Revokable} that may be used to revoke (undo) this operation later.
      */
-    public virtual async void archive_single_email_async(Geary.EmailIdentifier email_id,
+    public virtual async Geary.Revokable? archive_single_email_async(Geary.EmailIdentifier email_id,
         Cancellable? cancellable = null) throws Error {
         Gee.ArrayList<Geary.EmailIdentifier> ids = new Gee.ArrayList<Geary.EmailIdentifier>();
         ids.add(email_id);
         
-        yield archive_email_async(ids, cancellable);
+        return yield archive_email_async(ids, cancellable);
     }
 }
 
diff --git a/src/engine/api/geary-folder-supports-move.vala b/src/engine/api/geary-folder-supports-move.vala
index 58ededf..e76062f 100644
--- a/src/engine/api/geary-folder-supports-move.vala
+++ b/src/engine/api/geary-folder-supports-move.vala
@@ -19,8 +19,10 @@ public interface Geary.FolderSupport.Move : Geary.Folder {
      * way but will return success.
      *
      * The { link Geary.Folder} must be opened prior to attempting this operation.
+     *
+     * @returns A { link Geary.Revokable} that may be used to revoke (undo) this operation later.
      */
-    public abstract async void move_email_async(Gee.List<Geary.EmailIdentifier> to_move,
+    public abstract async Geary.Revokable? move_email_async(Gee.List<Geary.EmailIdentifier> to_move,
         Geary.FolderPath destination, Cancellable? cancellable = null) throws Error;
 }
 
diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala 
b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
index 16e5529..567c9a0 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
@@ -17,9 +17,21 @@ private class Geary.ImapEngine.GmailFolder : MinimalFolder, FolderSupport.Archiv
         return yield base.create_email_async(rfc822, flags, date_received, id, cancellable);
     }
     
-    public async void archive_email_async(Gee.List<Geary.EmailIdentifier> email_ids,
+    public async Geary.Revokable? archive_email_async(Gee.List<Geary.EmailIdentifier> email_ids,
         Cancellable? cancellable = null) throws Error {
+        // Use move_email_async("All Mail") here; Gmail will do the right thing and report
+        // it was copied with the pre-existing All Mail UID (in other words, no actual copy is
+        // performed).  This allows for undoing an archive with the same code path as a move.
+        Geary.Folder? all_mail = account.get_special_folder(Geary.SpecialFolderType.ALL_MAIL);
+        if (all_mail != null)
+            return yield move_email_async(email_ids, all_mail.path, cancellable);
+        
+        // although this shouldn't happen, fall back on our traditional archive, which is simply
+        // to remove the message from this label
+        message("%s: Unable to perform revokable archive: All Mail not found", to_string());
         yield expunge_email_async(email_ids, cancellable);
+        
+        return null;
     }
     
     public async void remove_email_async(Gee.List<Geary.EmailIdentifier> email_ids,
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index ff441f2..f6dd1dd 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -535,6 +535,9 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
      * not be reflected in the local database unless there's a separate connection to the server
      * that is notified or detects these changes.
      *
+     * The returned Folder must be opened prior to use and closed once completed.  ''Leaving a
+     * Folder open will cause a connection leak.''
+     *
      * It is not recommended this object be held open long-term, or that its status or notifications
      * be directly written to the database unless you know exactly what you're doing.  ''Caveat
      * implementor.''
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 0f5213d..30ed709 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1297,18 +1297,25 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         return copy.destination_uids.size > 0 ? copy.destination_uids : null;
     }
 
-    public virtual async void move_email_async(Gee.List<Geary.EmailIdentifier> to_move,
+    public virtual async Geary.Revokable? move_email_async(Gee.List<Geary.EmailIdentifier> to_move,
         Geary.FolderPath destination, Cancellable? cancellable = null) throws Error {
         check_open("move_email_async");
         check_ids("move_email_async", to_move);
         
         // watch for moving to this folder, which is treated as a no-op
         if (destination.equal_to(path))
-            return;
+            return null;
         
         MoveEmail move = new MoveEmail(this, (Gee.List<ImapDB.EmailIdentifier>) to_move, destination);
         replay_queue.schedule(move);
+        
         yield move.wait_for_ready_async(cancellable);
+        
+        // If no COPYUIDs returned, can't revoke this operation
+        if (move.destination_uids.size == 0)
+            return null;
+        
+        return new RevokableMove(_account, path, destination, move.destination_uids);
     }
     
     private void on_email_flags_changed(Gee.Map<Geary.EmailIdentifier, Geary.EmailFlags> changed) {
diff --git a/src/engine/imap-engine/imap-engine-revokable-move.vala 
b/src/engine/imap-engine/imap-engine-revokable-move.vala
new file mode 100644
index 0000000..59fbacf
--- /dev/null
+++ b/src/engine/imap-engine/imap-engine-revokable-move.vala
@@ -0,0 +1,123 @@
+/* Copyright 2014 Yorba Foundation
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later).  See the COPYING file in this distribution.
+ */
+
+private class Geary.ImapEngine.RevokableMove : Revokable {
+    private GenericAccount account;
+    private FolderPath original_source;
+    private FolderPath original_dest;
+    private Gee.Set<Imap.UID> destination_uids;
+    
+    /**
+     * Supplied EmailIdentifiers *must* be loaded with UIDs of the messages on the *destination*
+     * folder.  Do *not* merely stuff in here the EmailIdentifier from the source folder.
+     */
+    public RevokableMove(GenericAccount account, FolderPath original_source, FolderPath original_dest,
+        Gee.Set<Imap.UID> destination_uids) {
+        this.account = account;
+        this.original_source = original_source;
+        this.original_dest = original_dest;
+        this.destination_uids = destination_uids;
+        
+        account.folders_available_unavailable.connect(on_folders_available_unavailable);
+        account.email_removed.connect(on_folder_email_removed);
+    }
+    
+    ~RevokableMove() {
+        account.folders_available_unavailable.disconnect(on_folders_available_unavailable);
+        account.email_removed.disconnect(on_folder_email_removed);
+    }
+    
+    public override async bool revoke_async(Cancellable? cancellable) throws Error {
+        if (is_revoking)
+            throw new EngineError.ALREADY_OPEN("Already revoking operation");
+        
+        is_revoking = true;
+        try {
+            return yield internal_revoke_async(cancellable);
+        } finally {
+            is_revoking = false;
+        }
+    }
+    
+    private async bool internal_revoke_async(Cancellable? cancellable) throws Error {
+        // at this point, it's a one-shot deal: any error from here on out, or success, revoke
+        // is completed
+        can_revoke = false;
+        
+        // Use a detached Folder object, which bypasses synchronization on the destination folder
+        // for "quick" operations
+        Imap.Folder dest_folder = yield account.fetch_detached_folder_async(original_dest, cancellable);
+        yield dest_folder.open_async(cancellable);
+        
+        // open, revoke, close, ensuring the close and signal disconnect are performed in all cases
+        try {
+            // watch out for messages detected as gone when folder is opened
+            if (destination_uids.size > 0) {
+                Gee.List<Imap.MessageSet> msg_sets = Imap.MessageSet.uid_sparse(destination_uids);
+                
+                // copy the moved email back to its source
+                foreach (Imap.MessageSet msg_set in msg_sets)
+                    yield dest_folder.copy_email_async(msg_set, original_source, cancellable);
+                
+                // remove it from the destination in one fell swoop
+                yield dest_folder.remove_email_async(msg_sets, cancellable);
+                
+                // HACK:
+                // There's not a super-reliable way to wait until the delete of the message in this
+                // folder has round-tripped; could wait for the UID to be reported deleted, but it's
+                // possible in IMAP to delete a message not present in folder and not get an error
+                // (but also not be notified of its removal), and so waiting is a bad idea.  Don't
+                // want to close the folder immediately because that leaves the local remove_marker
+                // in place and the folder is treated as "dirty", causing a full renormalize to
+                // occur when next opened, which is expensive.  So: just wait a bit and give the
+                // server a chance to report the message is gone, leaving the db in a good state.
+                // Do this *after* marking can_revoke=false so client is notified that this object
+                // is now worthless.
+                yield Scheduler.sleep_async(5);
+            }
+        } finally {
+            // note that the Cancellable is not used
+            try {
+                yield dest_folder.close_async(null);
+            } catch (Error err) {
+                // ignored
+            }
+        }
+        
+        return can_revoke;
+    }
+    
+    private void on_folders_available_unavailable(Gee.List<Folder>? available, Gee.List<Folder>? 
unavailable) {
+        // look for either of the original folders going away
+        if (unavailable != null) {
+            foreach (Folder folder in unavailable) {
+                if (folder.path.equal_to(original_source) || folder.path.equal_to(original_dest)) {
+                    can_revoke = false;
+                    
+                    break;
+                }
+            }
+        }
+    }
+    
+    private void on_folder_email_removed(Folder folder, Gee.Collection<EmailIdentifier> ids) {
+        // one-way switch, and only interested in destination folder activity
+        if (!can_revoke || !folder.path.equal_to(original_dest))
+            return;
+        
+        // convert generic identifiers to UIDs
+        Gee.HashSet<Imap.UID> removed_uids = traverse<EmailIdentifier>(ids)
+            .cast_object<ImapDB.EmailIdentifier>()
+            .filter(id => id.uid == null)
+            .map<Imap.UID>(id => id.uid)
+            .to_hash_set();
+        
+        // otherwise, ability to revoke is best-effort
+        destination_uids.remove_all(removed_uids);
+        can_revoke = destination_uids.size > 0;
+    }
+}
+
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 6be265d..a9bb22d 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
@@ -5,6 +5,8 @@
  */
 
 private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation {
+    public Gee.Set<Imap.UID> destination_uids = new Gee.HashSet<Imap.UID>();
+    
     private MinimalFolder engine;
     private Gee.List<ImapDB.EmailIdentifier> to_move = new Gee.ArrayList<ImapDB.EmailIdentifier>();
     private Geary.FolderPath destination;
@@ -70,10 +72,15 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation
         Gee.List<Imap.MessageSet> msg_sets = Imap.MessageSet.uid_sparse(
             ImapDB.EmailIdentifier.to_uids(moved_ids));
         foreach (Imap.MessageSet msg_set in msg_sets) {
-            yield engine.remote_folder.copy_email_async(msg_set, destination, null);
-            yield engine.remote_folder.remove_email_async(msg_set.to_list(), null);
+            Gee.Map<Imap.UID, Imap.UID>? src_dst_uids = yield engine.remote_folder.copy_email_async(
+                msg_set, destination, null);
+            if (src_dst_uids != null)
+                destination_uids.add_all(src_dst_uids.values);
         }
         
+        // delete all once all copied
+        yield engine.remote_folder.remove_email_async(msg_sets, null);
+        
         return ReplayOperation.Status.COMPLETED;
     }
 


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