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

[Michael Gratton <mjog src gnome org>]

commit d0c8c25799a40713734507e6ef68afe7cabaa8c7
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

 meson_options.txt                                  |  4 +-
 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    | 50 ++++---------
 src/engine/outbox/outbox-folder.vala               |  1 -
 src/engine/smtp/smtp-client-service.vala           |  4 +-
 9 files changed, 107 insertions(+), 104 deletions(-)
---
diff --git a/meson_options.txt b/meson_options.txt
index 59535ebb..cb81e776 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,7 +1,7 @@
-option('valadoc', type: 'boolean', value: false, description: 'Whether to build the documentaton (requires 
valadoc).')
+option('valadoc', type: 'boolean', value: true, description: 'Whether to build the documentaton (requires 
valadoc).')
 option('contractor', type: 'boolean', value: false, description: 'Whether to install the contractor file 
(Elementary OS-specific).')
 option('poodle', type: 'boolean', value: true, description: 'Whether to apply the POODLE SSLv3 fix.')
-option('ref_tracking', type: 'boolean', value: false, description: 'Whether to use explicit reference 
tracking.')
+option('ref_tracking', type: 'boolean', value: true, description: 'Whether to use explicit reference 
tracking.')
 option('iso_639_xml', type: 'string', value: '', description: 'Full path to the ISO 639 XML file.')
 option('iso_3166_xml', type: 'string', value: '', description: 'Full path to the ISO 3166 XML file.')
 option('libunwind_optional', type: 'boolean', value: false, description: 'Determines if libunwind is 
required.')
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 b450f4ec..8c485fcd 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1409,46 +1409,20 @@ 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);
-
-        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 59c41ba0..377d7300 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);
     }
@@ -327,7 +327,7 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
         try {
             yield create.open_async(Geary.Folder.OpenFlags.NO_DELAY, cancellable);
             open = true;
-            yield create.create_email_async(message, null, null, null, cancellable);
+            yield create.create_email_async(message, null, null, cancellable);
         } finally {
             if (open) {
                 try {