[geary/mjog/493-undo-send: 10/25] Replace Geary.Account::send_email w/ Smtp.ClientService methods



commit fe835350ecf192b4ef0c952ab2ed415f6be4375a
Author: Michael Gratton <mike vee net>
Date:   Fri Nov 8 10:29:31 2019 +1100

    Replace Geary.Account::send_email w/ Smtp.ClientService methods
    
    Remove Geary.Account::send_email and the sending progress monitor in
    favour of exposing Geary.Smtp.ClientService as a public class and so
    allowing clients to acccess those symbols from Geary.Account::outgoing.
    
    Also make ::send_email a convenience class for doing a save/queue and
    expose those operations, so clients can managage the two seperately if
    desired (e.g. for undoing sending).
    
    Finally, make Outbox.Folder public since it's exposed by the SMTP
    service and maybe clients want to do something more detailed with it
    anyway?

 src/client/application/application-controller.vala | 33 +++++++---
 src/client/components/main-window.vala             | 28 +++++++--
 src/client/composer/composer-widget.vala           |  3 +-
 src/engine/api/geary-account.vala                  | 15 -----
 .../imap-engine/imap-engine-generic-account.vala   | 24 --------
 src/engine/outbox/outbox-folder.vala               | 10 ++-
 src/engine/smtp/smtp-client-service.vala           | 72 ++++++++++++++++++----
 test/engine/api/geary-account-mock.vala            |  6 --
 8 files changed, 112 insertions(+), 79 deletions(-)
---
diff --git a/src/client/application/application-controller.vala 
b/src/client/application/application-controller.vala
index 5340cb0a..6c2ff04d 100644
--- a/src/client/application/application-controller.vala
+++ b/src/client/application/application-controller.vala
@@ -894,11 +894,17 @@ public class Application.Controller : Geary.BaseObject {
                 on_account_status_notify
             );
 
-            account.email_sent.disconnect(on_sent);
             account.email_removed.disconnect(on_account_email_removed);
             account.folders_available_unavailable.disconnect(on_folders_available_unavailable);
-            account.sending_monitor.start.disconnect(on_sending_started);
-            account.sending_monitor.finish.disconnect(on_sending_finished);
+
+            Geary.Smtp.ClientService? smtp = (
+                account.outgoing as Geary.Smtp.ClientService
+            );
+            if (smtp != null) {
+                smtp.email_sent.disconnect(on_sent);
+                smtp.sending_monitor.start.disconnect(on_sending_started);
+                smtp.sending_monitor.finish.disconnect(on_sending_finished);
+            }
 
             // Now the account is not in the accounts map, reset any
             // status notifications for it
@@ -1157,11 +1163,17 @@ public class Application.Controller : Geary.BaseObject {
         // on_folders_available_unavailable expects it to be there
         this.accounts.set(account.information, context);
 
-        account.email_sent.connect(on_sent);
         account.email_removed.connect(on_account_email_removed);
         account.folders_available_unavailable.connect(on_folders_available_unavailable);
-        account.sending_monitor.start.connect(on_sending_started);
-        account.sending_monitor.finish.connect(on_sending_finished);
+
+        Geary.Smtp.ClientService? smtp = (
+            account.outgoing as Geary.Smtp.ClientService
+        );
+        if (smtp != null) {
+            smtp.email_sent.connect(on_sent);
+            smtp.sending_monitor.start.connect(on_sending_started);
+            smtp.sending_monitor.finish.connect(on_sending_finished);
+        }
 
         bool retry = false;
         do {
@@ -1635,16 +1647,21 @@ public class Application.Controller : Geary.BaseObject {
         }
     }
 
-    private void on_sent(Geary.Account account, Geary.RFC822.Message sent) {
         // Translators: The label for an in-app notification. The
         // string substitution is a list of recipients of the email.
+    private void on_sent(Geary.Smtp.ClientService service,
+                         Geary.RFC822.Message sent) {
         string message = _(
             "Successfully sent mail to %s."
         ).printf(Util.Email.to_short_recipient_display(sent));
         Components.InAppNotification notification =
             new Components.InAppNotification(message);
         this.main_window.add_notification(notification);
-        this.plugin_manager.notifications.email_sent(account, sent);
+
+        AccountContext? context = this.accounts.get(service.account);
+        if (context != null) {
+            this.plugin_manager.notifications.email_sent(context.account, sent);
+        }
     }
 
     // Returns a list of composer windows for an account, or null if none.
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index 5fef1733..876df1a7 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -1299,18 +1299,34 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
 
     private void on_account_available(Geary.AccountInformation account) {
         try {
-            this.progress_monitor.add(this.application.engine.get_account_instance(account).opening_monitor);
-            this.progress_monitor.add(this.application.engine.get_account_instance(account).sending_monitor);
-        } catch (Error e) {
+            Geary.Account? engine = this.application.engine.get_account_instance(account);
+            if (engine != null) {
+                this.progress_monitor.add(engine.opening_monitor);
+                Geary.Smtp.ClientService? smtp = (
+                    engine.outgoing as Geary.Smtp.ClientService
+                );
+                if (smtp != null) {
+                    this.progress_monitor.add(smtp.sending_monitor);
+                }
+            }
+        } catch (GLib.Error e) {
             debug("Could not access account progress monitors: %s", e.message);
         }
     }
 
     private void on_account_unavailable(Geary.AccountInformation account) {
         try {
-            
this.progress_monitor.remove(this.application.engine.get_account_instance(account).opening_monitor);
-            
this.progress_monitor.remove(this.application.engine.get_account_instance(account).sending_monitor);
-        } catch (Error e) {
+            Geary.Account? engine = this.application.engine.get_account_instance(account);
+            if (engine != null) {
+                this.progress_monitor.remove(engine.opening_monitor);
+                Geary.Smtp.ClientService? smtp = (
+                    engine.outgoing as Geary.Smtp.ClientService
+                );
+                if (smtp != null) {
+                    this.progress_monitor.remove(smtp.sending_monitor);
+                }
+            }
+        } catch (GLib.Error e) {
             debug("Could not access account progress monitors: %s", e.message);
         }
     }
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 828feff1..e16891af 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -1389,7 +1389,8 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
         // Perform send.
         try {
             yield this.editor.clean_content();
-            yield this.account.send_email_async(yield get_composed_email());
+            yield ((Geary.Smtp.ClientService) this.account.outgoing)
+            .send_email(yield get_composed_email(), null);
         } catch (Error e) {
             GLib.message("Error sending email: %s", e.message);
         }
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index 9d4040ae..e5a0fbd5 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -135,14 +135,12 @@ public abstract class Geary.Account : BaseObject, Loggable {
     public Geary.ProgressMonitor db_upgrade_monitor { get; protected set; }
     public Geary.ProgressMonitor db_vacuum_monitor { get; protected set; }
     public Geary.ProgressMonitor opening_monitor { get; protected set; }
-    public Geary.ProgressMonitor sending_monitor { get; protected set; }
 
 
     public signal void opened();
 
     public signal void closed();
 
-    public signal void email_sent(Geary.RFC822.Message rfc822);
 
     /**
      * Emitted to notify the client that some problem has occurred.
@@ -402,15 +400,6 @@ public abstract class Geary.Account : BaseObject, Loggable {
     public abstract async Geary.Folder get_required_special_folder_async(Geary.SpecialFolderType special,
         Cancellable? cancellable = null) throws Error;
 
-    /**
-     * Submits a ComposedEmail for delivery.  Messages may be scheduled for later delivery or immediately
-     * sent.  Subscribe to the "email-sent" signal to be notified of delivery.  Note that that signal
-     * does not return the ComposedEmail object but an RFC822-formatted object.  Allowing for the
-     * subscriber to attach some kind of token for later comparison is being considered.
-     */
-    public abstract async void send_email_async(Geary.ComposedEmail composed, Cancellable? cancellable = 
null)
-        throws Error;
-
     /**
      * Search the local account for emails referencing a Message-ID value
      * (which can appear in the Message-ID header itself, as well as the
@@ -557,10 +546,6 @@ public abstract class Geary.Account : BaseObject, Loggable {
         email_flags_changed(folder, flag_map);
     }
 
-    protected virtual void notify_email_sent(RFC822.Message message) {
-        email_sent(message);
-    }
-
     /** Fires a {@link report_problem} signal for this account. */
     protected virtual void notify_report_problem(ProblemReport report) {
         report_problem(report);
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index ff58e4d7..b790d647 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -95,7 +95,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         this.imap = imap;
 
         smtp.outbox = new Outbox.Folder(this, local_folder_root, local);
-        smtp.email_sent.connect(on_email_sent);
         smtp.report_problem.connect(notify_report_problem);
         smtp.set_loggable_parent(this);
         this.smtp = smtp;
@@ -108,7 +107,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
          );
 
         this.opening_monitor = new ReentrantProgressMonitor(Geary.ProgressType.ACTIVITY);
-        this.sending_monitor = this.smtp.sending_monitor;
         this.search_upgrade_monitor = local.search_index_monitor;
         this.db_upgrade_monitor = local.upgrade_monitor;
         this.db_vacuum_monitor = local.vacuum_monitor;
@@ -493,28 +491,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         return folder;
     }
 
-    public override async void send_email_async(Geary.ComposedEmail composed,
-                                                GLib.Cancellable? cancellable = null)
-        throws GLib.Error {
-        check_open();
-
-        // XXX work out what our public IP address is somehow and use
-        // that in preference to the sender's domain
-        string domain = composed.sender != null
-            ? composed.sender.domain
-            : this.information.primary_mailbox.domain;
-        Geary.RFC822.Message rfc822 =
-            yield new Geary.RFC822.Message.from_composed_email(
-                composed, GMime.utils_generate_message_id(domain), cancellable
-            );
-
-        yield this.smtp.queue_email(rfc822, cancellable);
-    }
-
-    private void on_email_sent(Geary.RFC822.Message rfc822) {
-        notify_email_sent(rfc822);
-    }
-
     private ImapDB.EmailIdentifier check_id(Geary.EmailIdentifier id) throws EngineError {
         ImapDB.EmailIdentifier? imapdb_id = id as ImapDB.EmailIdentifier;
         if (imapdb_id == null)
diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala
index eb58c1ce..6501a262 100644
--- a/src/engine/outbox/outbox-folder.vala
+++ b/src/engine/outbox/outbox-folder.vala
@@ -7,9 +7,9 @@
  */
 
 /**
- * Local folder for storing outgoing mail.
+ * A folder for storing outgoing mail.
  */
-private class Geary.Outbox.Folder :
+public class Geary.Outbox.Folder :
     Geary.AbstractLocalFolder,
     Geary.FolderSupport.Create,
     Geary.FolderSupport.Mark,
@@ -54,7 +54,7 @@ private class Geary.Outbox.Folder :
      * Returns the path to this folder.
      *
      * This is always the child of the root given to the constructor,
-     * with the name given by @{link MAGIC_BASENAME}.
+     * with the name given by {@link MAGIC_BASENAME}.
      */
     public override FolderPath path {
         get {
@@ -81,9 +81,7 @@ private class Geary.Outbox.Folder :
     private int64 next_ordering = 0;
 
 
-    // Requires the Database from the get-go because it runs a background task that access it
-    // whether open or not
-    public Folder(Account account, FolderRoot root, ImapDB.Account local) {
+    internal Folder(Account account, FolderRoot root, ImapDB.Account local) {
         this._account = account;
         this._path = root.get_child(MAGIC_BASENAME, Trillian.TRUE);
         this.local = local;
diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala
index d22e4712..69071d00 100644
--- a/src/engine/smtp/smtp-client-service.vala
+++ b/src/engine/smtp/smtp-client-service.vala
@@ -12,13 +12,16 @@
  * This class maintains a queue of email messages to be delivered, and
  * opens SMTP connections to deliver queued messages as needed.
  */
-internal class Geary.Smtp.ClientService : Geary.ClientService {
+public class Geary.Smtp.ClientService : Geary.ClientService {
 
 
-    // Used solely for debugging, hence "(no subject)" not marked for translation
-    private static string message_subject(RFC822.Message message) {
-        return (message.subject != null && !String.is_empty(message.subject.to_string()))
-            ? message.subject.to_string() : "(no subject)";
+    // Used solely for debugging, hence "(no subject)" not marked for
+    // translation
+    private static string email_subject(EmailHeaderSet email) {
+        return (
+            email.subject != null && !String.is_empty(email.subject.to_string()))
+            ? email.subject.to_string()
+            : "(no subject)";
     }
 
 
@@ -79,17 +82,60 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
     }
 
     /**
-     * Saves and queues an email in the outbox for delivery.
+     * Saves and queues email for immediate delivery.
+     *
+     * This is a convenience method that calls {@link save_email} then
+     * {@link queue_email} with the resulting id.
      */
-    public async void queue_email(RFC822.Message rfc822,
-                                  GLib.Cancellable? cancellable)
+    public async void send_email(Geary.ComposedEmail composed,
+                                 GLib.Cancellable? cancellable)
         throws GLib.Error {
-        debug("Queuing message for sending: %s", message_subject(rfc822));
+        queue_email(yield save_email(composed, cancellable));
+    }
+
+    /**
+     * Saves a composed email in the outbox.
+     *
+     * This sets a suitable MessageID header for the message, then
+     * saves the updated message in {@link outbox}. Returns the
+     * identifier for the saved email, suitable for use with {@link
+     * queue_email}.
+     *
+     * @see send_email
+     */
+    public async EmailIdentifier save_email(Geary.ComposedEmail composed,
+                                            GLib.Cancellable? cancellable)
+        throws GLib.Error {
+        debug("Saving composed email: %s", email_subject(composed));
+
+        // XXX work out what our public IP address is somehow and use
+        // that in preference to the sender's domain
+        string domain = composed.sender != null
+            ? composed.sender.domain
+            : this.account.primary_mailbox.domain;
+        Geary.RFC822.Message rfc822 =
+            yield new Geary.RFC822.Message.from_composed_email(
+                composed, GMime.utils_generate_message_id(domain), cancellable
+            );
 
         EmailIdentifier id = yield this.outbox.create_email_async(
             rfc822, null, null, cancellable
         );
-        this.outbox_queue.send(id);
+        debug("Saved composed email as %s", id.to_string());
+        return id;
+    }
+
+    /**
+     * Queues an email for immediate delivery.
+     *
+     * The given identifier must be for {@link outbox}, for example as
+     * given by {@link save_email}.
+     *
+     * @see send_email
+     */
+    public void queue_email(EmailIdentifier outbox_identifier) {
+        debug("Queuing email for sending: %s", outbox_identifier.to_string());
+        this.outbox_queue.send(outbox_identifier);
     }
 
     /** Starts the postie delivering messages. */
@@ -204,8 +250,8 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
         if (!email.email_flags.contains(EmailFlags.OUTBOX_SENT)) {
             RFC822.Message message = email.get_message();
             debug("Outbox postie: Sending \"%s\" (ID:%s)...",
-                  message_subject(message), email.id.to_string());
-            yield send_email(message, cancellable);
+                  email_subject(message), email.id.to_string());
+            yield send_email_internal(message, cancellable);
 
             // Mark as sent, so if there's a problem pushing up to
             // Sent, we don't retry sending. Don't pass the
@@ -243,7 +289,7 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
         yield this.outbox.remove_email_async(Collection.single(email.id), null);
     }
 
-    private async void send_email(Geary.RFC822.Message rfc822, Cancellable? cancellable)
+    private async void send_email_internal(Geary.RFC822.Message rfc822, Cancellable? cancellable)
         throws Error {
         Credentials? login = this.account.get_outgoing_credentials();
         if (login != null && !login.is_complete()) {
diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala
index 45c90774..1a1aeeb1 100644
--- a/test/engine/api/geary-account-mock.vala
+++ b/test/engine/api/geary-account-mock.vala
@@ -181,12 +181,6 @@ public class Geary.MockAccount : Account, MockObject {
         );
     }
 
-    public override async void send_email_async(ComposedEmail composed,
-                                                Cancellable? cancellable = null)
-        throws Error {
-        void_call("send_email_async", {composed, cancellable});
-    }
-
     public override async Gee.MultiMap<Email,FolderPath?>?
         local_search_message_id_async(RFC822.MessageID message_id,
                                       Email.Field requested_fields,


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