[geary/wip/17-noisy-problem-reports: 11/12] Tidy up Imap.Command API a bit

[Michael Gratton <mjog src gnome org>]

commit b74d600e07b4854c21d0c5321eb8244001a1bf5f
Author: Michael Gratton <mike vee net>
Date:   Tue Jan 1 22:11:53 2019 +1100

    Tidy up Imap.Command API a bit
    
    Low-level uses of Command (e.g. by ClientConnection) are now internal,
    while high level call uses (e.g. by Imap.ClientSession, etc) remain
    public.

 .../imap/command/imap-authenticate-command.vala    | 31 +++++++--------
 src/engine/imap/command/imap-command.vala          | 44 +++++++++++-----------
 src/engine/imap/command/imap-idle-command.vala     | 24 ++++++------
 3 files changed, 50 insertions(+), 49 deletions(-)
---
diff --git a/src/engine/imap/command/imap-authenticate-command.vala 
b/src/engine/imap/command/imap-authenticate-command.vala
index a50b5159..acf49f3d 100644
--- a/src/engine/imap/command/imap-authenticate-command.vala
+++ b/src/engine/imap/command/imap-authenticate-command.vala
@@ -40,18 +40,25 @@ public class Geary.Imap.AuthenticateCommand : Command {
         this(OAUTH2_METHOD, encoded_token);
     }
 
-    public override async void send(Serializer ser,
-                                    GLib.Cancellable cancellable)
+    internal override async void send(Serializer ser,
+                                      GLib.Cancellable cancellable)
         throws GLib.Error {
         yield base.send(ser, cancellable);
         this.serialised = true;
 
         // Need to manually flush here since the connection will be
-        // waiting this to complete before do so.
+        // waiting for all pending commands to complete before
+        // flushing it itself
         yield ser.flush_stream(cancellable);
     }
 
-    public override async void send_wait(Serializer ser,
+    public override string to_string() {
+        return "%s %s %s <token>".printf(
+            tag.to_string(), this.name, this.method
+        );
+    }
+
+    internal override async void send_wait(Serializer ser,
                                          GLib.Cancellable cancellable)
         throws GLib.Error {
         // Wait to either get a response or a continuation request
@@ -65,18 +72,13 @@ public class Geary.Imap.AuthenticateCommand : Command {
         yield wait_until_complete(cancellable);
     }
 
-    public override void cancel_send() {
-        base.cancel_send();
-        this.error_cancellable.cancel();
-    }
-
-    public override void completed(StatusResponse new_status)
+    internal override void completed(StatusResponse new_status)
         throws ImapError {
         this.error_lock.blind_notify();
         base.completed(new_status);
     }
 
-    public override void continuation_requested(ContinuationResponse response)
+    internal override void continuation_requested(ContinuationResponse response)
         throws ImapError {
         if (!this.serialised) {
             // Allow any args sent as literals to be processed
@@ -104,10 +106,9 @@ public class Geary.Imap.AuthenticateCommand : Command {
         }
     }
 
-    public override string to_string() {
-        return "%s %s %s <token>".printf(
-            tag.to_string(), this.name, this.method
-        );
+    protected override void cancel_send() {
+        base.cancel_send();
+        this.error_cancellable.cancel();
     }
 
 }
diff --git a/src/engine/imap/command/imap-command.vala b/src/engine/imap/command/imap-command.vala
index 5df2d357..a77bfa55 100644
--- a/src/engine/imap/command/imap-command.vala
+++ b/src/engine/imap/command/imap-command.vala
@@ -96,7 +96,7 @@ public class Geary.Imap.Command : BaseObject {
      * @see Tag
      */
     public Command(string name, string[]? args = null) {
-        tag = Tag.get_unassigned();
+        this.tag = Tag.get_unassigned();
         this.name = name;
         if (args != null) {
             foreach (string arg in args) {
@@ -116,13 +116,11 @@ public class Geary.Imap.Command : BaseObject {
     /**
      * Assign a Tag to this command, if currently unassigned.
      *
-     * Can only be called on a Command that holds an unassigned Tag.
-     * Thus, this can only be called once at most, and zero times if
-     * Command.assigned() was used to generate the Command.  Fires an
-     * assertion if either of these cases is true, or if the supplied
-     * Tag is unassigned.
+     * Can only be called on a Command that holds an unassigned tag,
+     * and hence this can only be called once at most. Throws an error
+     * if already assigned or if the supplied tag is unassigned.
      */
-    public void assign_tag(Tag new_tag) throws ImapError {
+    internal void assign_tag(Tag new_tag) throws ImapError {
         if (this.tag.is_assigned()) {
             throw new ImapError.SERVER_ERROR(
                 "%s: Command tag is already assigned", to_brief_string()
@@ -146,8 +144,8 @@ public class Geary.Imap.Command : BaseObject {
      * required, this method will yield until a command continuation
      * has been received, when it will resume the same process.
      */
-    public virtual async void send(Serializer ser,
-                                   GLib.Cancellable cancellable)
+    internal virtual async void send(Serializer ser,
+                                     GLib.Cancellable cancellable)
         throws GLib.Error {
         this.response_timer.start();
         this.tag.serialize(ser, cancellable);
@@ -199,8 +197,8 @@ public class Geary.Imap.Command : BaseObject {
      * Most commands will not need to override this, and it by default
      * does nothing.
      */
-    public virtual async void send_wait(Serializer ser,
-                                        GLib.Cancellable cancellable)
+    internal virtual async void send_wait(Serializer ser,
+                                          GLib.Cancellable cancellable)
         throws GLib.Error {
         // Nothing to do by default
     }
@@ -211,7 +209,7 @@ public class Geary.Imap.Command : BaseObject {
      * When this method is called, all locks will be released,
      * including {@link wait_until_complete}.
      */
-    public virtual void cancel_command() {
+    internal virtual void cancel_command() {
         cancel_send();
         this.response_timer.reset();
         this.complete_lock.blind_notify();
@@ -231,13 +229,20 @@ public class Geary.Imap.Command : BaseObject {
         check_status(true);
     }
 
+    public virtual string to_string() {
+        string args = this.args.to_string();
+        return (Geary.String.is_empty(args))
+            ? "%s %s".printf(this.tag.to_string(), this.name)
+            : "%s %s %s".printf(this.tag.to_string(), this.name, args);
+    }
+
     /**
      * Called when a tagged status response is received for this command.
      *
      * This will update the command's {@link status} property, then
-     * throw an error it does not indicate a successful completion.
+     * throw an error if it does not indicate a successful completion.
      */
-    public virtual void completed(StatusResponse new_status)
+    internal virtual void completed(StatusResponse new_status)
         throws ImapError {
         if (this.status != null) {
             cancel_send();
@@ -260,7 +265,7 @@ public class Geary.Imap.Command : BaseObject {
     /**
      * Called when tagged server data is received for this command.
      */
-    public virtual void data_received(ServerData data)
+    internal virtual void data_received(ServerData data)
         throws ImapError {
         if (this.status != null) {
             cancel_send();
@@ -281,7 +286,7 @@ public class Geary.Imap.Command : BaseObject {
      * {@link send} is waiting to send a literal, it will do so
      * now.
      */
-    public virtual void
+    internal virtual void
         continuation_requested(ContinuationResponse continuation)
         throws ImapError {
         if (this.status != null) {
@@ -304,13 +309,6 @@ public class Geary.Imap.Command : BaseObject {
         this.literal_spinlock.blind_notify();
     }
 
-    public virtual string to_string() {
-        string args = this.args.to_string();
-        return (Geary.String.is_empty(args))
-            ? "%s %s".printf(this.tag.to_string(), this.name)
-            : "%s %s %s".printf(this.tag.to_string(), this.name, args);
-    }
-
     /**
      * Cancels any existing serialisation in progress.
      *
diff --git a/src/engine/imap/command/imap-idle-command.vala b/src/engine/imap/command/imap-idle-command.vala
index ce3232a3..2239838f 100644
--- a/src/engine/imap/command/imap-idle-command.vala
+++ b/src/engine/imap/command/imap-idle-command.vala
@@ -30,13 +30,14 @@ public class Geary.Imap.IdleCommand : Command {
         this.exit_lock = new Geary.Nonblocking.Spinlock(this.exit_cancellable);
     }
 
+    /** Causes the idle command to exit, if currently executing. **/
     public void exit_idle() {
         this.exit_lock.blind_notify();
     }
 
     /** Waits after serialisation has completed for {@link exit_idle}. */
-    public override async void send(Serializer ser,
-                                    GLib.Cancellable cancellable)
+    internal override async void send(Serializer ser,
+                                      GLib.Cancellable cancellable)
         throws GLib.Error {
         // Need to manually flush here since Dovecot doesn't like
         // getting IDLE in the same buffer as other commands.
@@ -46,12 +47,13 @@ public class Geary.Imap.IdleCommand : Command {
         this.serialised = true;
 
         // Need to manually flush again since the connection will be
-        // waiting this to complete before do so.
+        // waiting for all pending commands to complete before
+        // flushing it itself
         yield ser.flush_stream(cancellable);
     }
 
-    public override async void send_wait(Serializer ser,
-                                         GLib.Cancellable cancellable)
+    internal override async void send_wait(Serializer ser,
+                                           GLib.Cancellable cancellable)
         throws GLib.Error {
         // Wait for exit_idle() to be called, the server to send a
         // status response, or everything to be cancelled.
@@ -72,12 +74,7 @@ public class Geary.Imap.IdleCommand : Command {
         yield wait_until_complete(cancellable);
     }
 
-    public override void cancel_send() {
-        base.cancel_send();
-        this.exit_cancellable.cancel();
-    }
-
-    public override void continuation_requested(ContinuationResponse response)
+    internal override void continuation_requested(ContinuationResponse response)
         throws ImapError {
         if (!this.serialised) {
             // Allow any args sent as literals to be processed
@@ -91,4 +88,9 @@ public class Geary.Imap.IdleCommand : Command {
         }
     }
 
+    protected override void cancel_send() {
+        base.cancel_send();
+        this.exit_cancellable.cancel();
+    }
+
 }