[geary/wip/76-gmail-drafts: 72/72] Don't support removing an email when creating another

[Michael Gratton <mjog src gnome org>]

commit 942a4fa64f8e2ed5a9fd90ee422077e4732142b8
Author: Michael Gratton <mike vee net>
Date:   Sun Aug 11 16:41:27 2019 +1000

    Don't support removing an email when creating another
    
    Remove the `id` param from FolderSupport.Create::create_email_async.
    Fix up all implementations and call sites. Update App.DraftManager to
    explicitly handle removing the old draft after creating the new one.
    
    This ensures the custom folder delete support gets used and in
    particular, ensures that old drafts under Gmail disappear ASAP.
    
    Fixes #76

 src/engine/api/geary-folder-supports-create.vala   |  7 +-
 src/engine/app/app-draft-manager.vala              | 82 +++++++++++++---------
 .../gmail/imap-engine-gmail-drafts-folder.vala     | 37 ++++++----
 .../gmail/imap-engine-gmail-folder.vala            | 13 ++--
 .../imap-engine/imap-engine-generic-folder.vala    | 13 ++--
 .../imap-engine/imap-engine-minimal-folder.vala    | 54 ++++----------
 src/engine/outbox/outbox-folder.vala               |  1 -
 src/engine/smtp/smtp-client-service.vala           |  4 +-
 8 files changed, 108 insertions(+), 103 deletions(-)
---
diff --git a/src/engine/api/geary-folder-supports-create.vala 
b/src/engine/api/geary-folder-supports-create.vala
index b04b2080..664e9f6c 100644
--- a/src/engine/api/geary-folder-supports-create.vala
+++ b/src/engine/api/geary-folder-supports-create.vala
@@ -2,7 +2,7 @@
  * Copyright 2016 Software Freedom Conservancy Inc.
  *
  * This software is licensed under the GNU Lesser General Public License
- * (version 2.1 or later).  See the COPYING file in this distribution.
+ * (version 2.1 or later). See the COPYING file in this distribution.
  */
 
 /**
@@ -30,17 +30,12 @@ public interface Geary.FolderSupport.Create : Folder {
      * time to be set when saved.  Like EmailFlags, this is optional
      * if not applicable.
      *
-     * If an id is passed, this will replace the existing message by
-     * deleting it after the new message is created.  The new
-     * message's ID is returned.
-     *
      * @see FolderProperties.create_never_returns_id
      */
     public abstract async EmailIdentifier?
         create_email_async(RFC822.Message rfc822,
                            EmailFlags? flags,
                            DateTime? date_received,
-                           EmailIdentifier? id,
                            GLib.Cancellable? cancellable = null)
         throws GLib.Error;
 
diff --git a/src/engine/app/app-draft-manager.vala b/src/engine/app/app-draft-manager.vala
index 620453ba..1ff0fb4a 100644
--- a/src/engine/app/app-draft-manager.vala
+++ b/src/engine/app/app-draft-manager.vala
@@ -408,8 +408,13 @@ public class Geary.App.DraftManager : BaseObject {
             return false;
 
         // make sure there's a folder to work with
-        if (drafts_folder == null || drafts_folder.get_open_state() == Folder.OpenState.CLOSED) {
-            fatal(new EngineError.SERVER_UNAVAILABLE("%s: premature drafts folder close", to_string()));
+        if (this.drafts_folder == null ||
+            this.drafts_folder.get_open_state() == CLOSED) {
+            fatal(
+                new EngineError.SERVER_UNAVAILABLE(
+                    "%s: premature drafts folder close", to_string()
+                )
+            );
 
             return false;
         }
@@ -417,44 +422,48 @@ public class Geary.App.DraftManager : BaseObject {
         // at this point, only operation left is PUSH
         assert(op.op_type == OperationType.PUSH);
 
-        draft_state = DraftState.STORING;
-
-        // delete old draft for all PUSHes: best effort ... since create_email_async() will handle
-        // replacement in a transactional-kinda-way, only outright delete if not using create
-        if (current_draft_id != null && op.draft == null) {
-            bool success = false;
-            try {
-                yield remove_support.remove_email_async(
-                    iterate<EmailIdentifier>(current_draft_id).to_array_list());
-                success = true;
-            } catch (Error err) {
-                debug("%s: Unable to remove existing draft %s: %s", to_string(), 
current_draft_id.to_string(),
-                    err.message);
-            }
-
-            // always clear draft id (assuming that retrying a failed remove is unnecessary), but
-            // only signal the discard if it actually was removed
-            current_draft_id = null;
-            if (success)
-                notify_discarded();
-        }
+        this.draft_state = DraftState.STORING;
 
         // if draft supplied, save it
         if (op.draft != null) {
             try {
-                current_draft_id = yield create_support.create_email_async(op.draft, op.flags,
-                    op.date_received, current_draft_id, null);
-
-                draft_state = DraftState.STORED;
+                EmailIdentifier? old_id = this.current_draft_id;
+                this.current_draft_id =
+                    yield this.create_support.create_email_async(
+                        op.draft,
+                        op.flags,
+                        op.date_received,
+                        null
+                    );
+                yield this.remove_support.remove_email_async(
+                    Collection.single(old_id), null
+                );
+
+                this.draft_state = DraftState.STORED;
                 notify_stored(op.draft);
-            } catch (Error err) {
-                draft_state = DraftState.ERROR;
-
-                // notify subscribers
+            } catch (GLib.Error err) {
+                this.draft_state = DraftState.ERROR;
                 draft_failed(op.draft, err);
             }
         } else {
-            draft_state = DraftState.NOT_STORED;
+            this.draft_state = DraftState.NOT_STORED;
+
+            // Delete the old draft if present so it's not hanging
+            // around
+            if (this.current_draft_id != null) {
+                try {
+                    yield this.remove_support.remove_email_async(
+                        Collection.single(this.current_draft_id),
+                        null
+                    );
+                    notify_discarded();
+                } catch (GLib.Error err) {
+                    warning("%s: Unable to remove existing draft %s: %s",
+                            to_string(),
+                            current_draft_id.to_string(),
+                            err.message);
+                }
+            }
         }
 
         return true;
@@ -463,5 +472,12 @@ public class Geary.App.DraftManager : BaseObject {
     public string to_string() {
         return "%s DraftManager".printf(account.to_string());
     }
-}
 
+    private async void save_draft(EmailIdentifier id) {
+
+    }
+
+    private async void remove_draft(EmailIdentifier id) {
+
+    }
+}
diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-drafts-folder.vala 
b/src/engine/imap-engine/gmail/imap-engine-gmail-drafts-folder.vala
index b86465c2..ede8622a 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-drafts-folder.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-drafts-folder.vala
@@ -1,32 +1,41 @@
-/* Copyright 2016 Software Freedom Conservancy Inc.
+/*
+ * Copyright 2016 Software Freedom Conservancy Inc.
  *
  * This software is licensed under the GNU Lesser General Public License
- * (version 2.1 or later).  See the COPYING file in this distribution.
+ * (version 2.1 or later). See the COPYING file in this distribution.
  */
 
 /**
- * Gmail's Drafts folder supports basic operations as well as true removal of messages and creating
- * new ones (IMAP APPEND).
+ * A draft folder for Gmail.
+ *
+ * Gmail's drafts folders supports basic operations as well as true
+ * removal of messages and creating new ones (IMAP APPEND).
  */
+private class Geary.ImapEngine.GmailDraftsFolder :
+    MinimalFolder, FolderSupport.Create, FolderSupport.Remove {
 
-private class Geary.ImapEngine.GmailDraftsFolder : MinimalFolder, FolderSupport.Create,
-    FolderSupport.Remove {
     public GmailDraftsFolder(GmailAccount account,
                              ImapDB.Folder local_folder,
                              SpecialFolderType special_folder_type) {
-        base (account, local_folder, special_folder_type);
+        base(account, local_folder, special_folder_type);
     }
 
-    public new async Geary.EmailIdentifier? create_email_async(
-        RFC822.Message rfc822, Geary.EmailFlags? flags, DateTime? date_received,
-        Geary.EmailIdentifier? id, Cancellable? cancellable = null) throws Error {
-        return yield base.create_email_async(rfc822, flags, date_received, id, cancellable);
+    public new async EmailIdentifier?
+        create_email_async(RFC822.Message rfc822,
+                           EmailFlags? flags,
+                           DateTime? date_received,
+                           GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
+        return yield base.create_email_async(
+            rfc822, flags, date_received, cancellable
+        );
     }
 
-    public async void remove_email_async(
-        Gee.Collection<Geary.EmailIdentifier> email_ids,
-        GLib.Cancellable? cancellable = null)
+    public async void
+        remove_email_async(Gee.Collection<EmailIdentifier> email_ids,
+                           GLib.Cancellable? cancellable = null)
         throws GLib.Error {
         yield GmailFolder.true_remove_email_async(this, email_ids, cancellable);
     }
+
 }
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 4744f67c..cab40317 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
@@ -13,10 +13,15 @@ private class Geary.ImapEngine.GmailFolder : MinimalFolder, FolderSupport.Archiv
         base (account, local_folder, special_folder_type);
     }
 
-    public new async Geary.EmailIdentifier? create_email_async(
-        RFC822.Message rfc822, Geary.EmailFlags? flags, DateTime? date_received,
-        Geary.EmailIdentifier? id, Cancellable? cancellable = null) throws Error {
-        return yield base.create_email_async(rfc822, flags, date_received, id, cancellable);
+    public new async EmailIdentifier?
+        create_email_async(RFC822.Message rfc822,
+                           EmailFlags? flags,
+                           DateTime? date_received,
+                           GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
+        return yield base.create_email_async(
+            rfc822, flags, date_received, cancellable
+        );
     }
 
     public async Geary.Revokable?
diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala 
b/src/engine/imap-engine/imap-engine-generic-folder.vala
index 819eac9d..b9a28b22 100644
--- a/src/engine/imap-engine/imap-engine-generic-folder.vala
+++ b/src/engine/imap-engine/imap-engine-generic-folder.vala
@@ -48,9 +48,14 @@ private class Geary.ImapEngine.GenericFolder : MinimalFolder,
         yield expunge_all_async(cancellable);
     }
 
-    public new async Geary.EmailIdentifier? create_email_async(RFC822.Message rfc822,
-        Geary.EmailFlags? flags, DateTime? date_received, Geary.EmailIdentifier? id,
-        Cancellable? cancellable = null) throws Error {
-        return yield base.create_email_async(rfc822, flags, date_received, id, cancellable);
+    public new async EmailIdentifier?
+        create_email_async(RFC822.Message rfc822,
+                           EmailFlags? flags,
+                           DateTime? date_received,
+                           GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
+        return yield base.create_email_async(
+            rfc822, flags, date_received, cancellable
+        );
     }
 }
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 3f3bc1fe..18eee9bb 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1458,58 +1458,34 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         return earliest;
     }
 
-    protected async Geary.EmailIdentifier? create_email_async(RFC822.Message rfc822,
-        Geary.EmailFlags? flags, DateTime? date_received, Geary.EmailIdentifier? id,
-        Cancellable? cancellable = null) throws Error {
+    protected async EmailIdentifier?
+        create_email_async(RFC822.Message rfc822,
+                           EmailFlags? flags,
+                           DateTime? date_received,
+                           GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
         check_open("create_email_async");
-        if (id != null)
-            check_id("create_email_async", id);
-
-        Error? cancel_error = null;
-        Geary.EmailIdentifier? ret = null;
-        try {
-            CreateEmail create = new CreateEmail(this, rfc822, flags, date_received, cancellable);
-            replay_queue.schedule(create);
-            yield create.wait_for_ready_async(cancellable);
-
-            ret = create.created_id;
-        } catch (Error e) {
-            if (e is IOError.CANCELLED)
-                cancel_error = e;
-            else
-                throw e;
-        }
-
-        Geary.FolderSupport.Remove? remove_folder = this as Geary.FolderSupport.Remove;
-
-        // Remove old message.
-        if (id != null && remove_folder != null)
-            yield remove_folder.remove_email_async(iterate<EmailIdentifier>(id).to_array_list());
-
-        // If the user cancelled the operation, throw the error here.
-        if (cancel_error != null)
-            throw cancel_error;
-
-        // If the caller cancelled during the remove operation, delete the newly created message to
-        // safely back out.
-        if (cancellable != null && cancellable.is_cancelled() && ret != null && remove_folder != null)
-            yield remove_folder.remove_email_async(iterate<EmailIdentifier>(ret).to_array_list());
+        CreateEmail op = new CreateEmail(
+            this, rfc822, flags, date_received, cancellable
+        );
+        replay_queue.schedule(op);
+        yield op.wait_for_ready_async(cancellable);
+        this._account.update_folder(this);
 
-        if (ret != null) {
+        if (op.created_id != null) {
             // Server returned a UID for the new message. It was saved
             // locally possibly before the server notified that the
             // message exists. As such, fetch any missing parts from
             // the remote to ensure it is properly filled in.
             yield list_email_by_id_async(
-                ret, 1, ALL, INCLUDING_ID, cancellable
+                op.created_id, 1, ALL, INCLUDING_ID, cancellable
             );
         } else {
             // The server didn't return a UID for the new email, so do
             // a sync now to ensure it shows up immediately.
             yield synchronise_remote(cancellable);
         }
-
-        return ret;
+        return op.created_id;
     }
 
     /** Fires a {@link marked_email_removed} signal for this folder. */
diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala
index 6f72194f..eb58c1ce 100644
--- a/src/engine/outbox/outbox-folder.vala
+++ b/src/engine/outbox/outbox-folder.vala
@@ -112,7 +112,6 @@ private class Geary.Outbox.Folder :
         create_email_async(RFC822.Message rfc822,
                            Geary.EmailFlags? flags,
                            GLib.DateTime? date_received,
-                           Geary.EmailIdentifier? id = null,
                            GLib.Cancellable? cancellable = null)
         throws GLib.Error {
         check_open();
diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala
index 51dc2f7f..d22e4712 100644
--- a/src/engine/smtp/smtp-client-service.vala
+++ b/src/engine/smtp/smtp-client-service.vala
@@ -87,7 +87,7 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
         debug("Queuing message for sending: %s", message_subject(rfc822));
 
         EmailIdentifier id = yield this.outbox.create_email_async(
-            rfc822, null, null, null, cancellable
+            rfc822, null, null, cancellable
         );
         this.outbox_queue.send(id);
     }
@@ -330,7 +330,7 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
         try {
             yield create.open_async(NO_DELAY, cancellable);
             open = true;
-            yield create.create_email_async(raw, null, null, null, cancellable);
+            yield create.create_email_async(raw, null, null, cancellable);
             yield wait_for_message(create, message, cancellable);
         } finally {
             if (open) {