[geary/wip/721828-undo-2: 2/2] End-to-end functionality, although not as snappy as we would like
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/721828-undo-2: 2/2] End-to-end functionality, although not as snappy as we would like
- Date: Wed, 14 Jan 2015 00:09:32 +0000 (UTC)
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]