[geary/mjog/logging-update: 3/8] Add new Geary.Logging.State object for snapshotting source state

[Michael Gratton <mjog src gnome org>]

commit 70dffd783a9f734780ec5b63dfb5471b226aaeb1
Author: Michael Gratton <mike vee net>
Date:   Mon Dec 2 11:38:55 2019 +1100

    Add new Geary.Logging.State object for snapshotting source state
    
    Add State object and Geary.Logging.State::to_logging_state factory
    method. Use this in LogRecord to print formatted log messages so that
    sources can log state that may change between being logged and the
    log message being displayed/saved.
    
    Provide a default implementation of Source::to_string that uses the
    instance's current state.
    
    Update implementing classes.

 src/engine/api/geary-account.vala                  |  7 +-
 src/engine/api/geary-client-service.vala           |  7 +-
 src/engine/api/geary-folder.vala                   |  8 +--
 src/engine/api/geary-logging.vala                  | 70 +++++--------------
 .../imap-engine/imap-engine-account-operation.vala | 20 +++---
 .../imap-engine/imap-engine-minimal-folder.vala    | 11 ++-
 src/engine/util/util-logging.vala                  | 79 +++++++++++++++++++++-
 7 files changed, 117 insertions(+), 85 deletions(-)
---
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index e4223cf7..4d5d6aad 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -474,11 +474,8 @@ public abstract class Geary.Account : BaseObject, Logging.Source {
         Gee.Collection<Geary.EmailIdentifier> ids, Cancellable? cancellable) throws Error;
 
     /** {@inheritDoc} */
-    public virtual string to_string() {
-        return "%s(%s)".printf(
-            this.get_type().name(),
-            this.information.id
-        );
+    public virtual Logging.State to_logging_state() {
+        return new Logging.State(this, this.information.id);
     }
 
     /** Fires a {@link opened} signal. */
diff --git a/src/engine/api/geary-client-service.vala b/src/engine/api/geary-client-service.vala
index c9dbcf29..b5bee4fc 100644
--- a/src/engine/api/geary-client-service.vala
+++ b/src/engine/api/geary-client-service.vala
@@ -293,11 +293,8 @@ public abstract class Geary.ClientService : BaseObject, Logging.Source {
     }
 
     /** {@inheritDoc} */
-    public virtual string to_string() {
-        return "%s(%s)".printf(
-            this.get_type().name(),
-            this.configuration.protocol.to_value()
-        );
+    public virtual Logging.State to_logging_state() {
+        return new Logging.State(this, this.configuration.protocol.to_value());
     }
 
     /** Sets the service's logging parent. */
diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala
index a7aeb632..f2762f09 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -696,12 +696,8 @@ public abstract class Geary.Folder : BaseObject, Logging.Source {
         Geary.Email.Field required_fields, ListFlags flags, Cancellable? cancellable = null) throws Error;
 
     /** {@inheritDoc} */
-    public virtual string to_string() {
-        return "%s(%s:%s)".printf(
-            this.get_type().name(),
-            this.account.information.id,
-            this.path.to_string()
-        );
+    public virtual Logging.State to_logging_state() {
+        return new Logging.State(this, this.path.to_string());
     }
 
 }
diff --git a/src/engine/api/geary-logging.vala b/src/engine/api/geary-logging.vala
index 146b58a5..f5b1e097 100644
--- a/src/engine/api/geary-logging.vala
+++ b/src/engine/api/geary-logging.vala
@@ -153,7 +153,7 @@ public class Record {
     /** The next log record in the buffer, if any. */
     public Record? next { get; internal set; default = null; }
 
-    private Source[] sources;
+    private State[] states;
     private bool filled = false;
     private bool old_log_api = false;
 
@@ -170,12 +170,13 @@ public class Record {
 
         // Since GLib.LogField only retains a weak ref to its value,
         // find and ref any values we wish to keep around.
-        this.sources = new Source[fields.length];
-        int source_count = 0;
+        this.states = new State[fields.length];
+        int state_count = 0;
         foreach (GLib.LogField field in fields) {
             switch (field.key) {
             case "GEARY_LOGGING_SOURCE":
-                this.sources[source_count++] = (Source) field.value;
+                this.states[state_count++] =
+                    ((Source) field.value).to_logging_state();
                 break;
 
             case "GEARY_FLAGS":
@@ -204,24 +205,7 @@ public class Record {
             }
         }
 
-        this.sources.length = source_count;
-    }
-
-    /** Returns the record's sources that aren't well-known. */
-    public Source[] get_other_sources() {
-        fill_well_known_sources();
-
-        Source[] copy = new Source[this.sources.length];
-        int count = 0;
-        foreach (Source source in this.sources) {
-            if (source != this.account &&
-                source != this.service &&
-                source != this.folder) {
-                copy[count++] = source;
-            }
-        }
-        copy.length = count;
-        return copy;
+        this.states.length = state_count;
     }
 
     /**
@@ -233,14 +217,14 @@ public class Record {
      */
     public void fill_well_known_sources() {
         if (!this.filled) {
-            foreach (Source source in this.sources) {
-                GLib.Type type = source.get_type();
+            foreach (unowned State state in this.states) {
+                GLib.Type type = state.source.get_type();
                 if (type.is_a(typeof(Account))) {
-                    this.account = (Account) source;
+                    this.account = (Account) state.source;
                 } else if (type.is_a(typeof(ClientService))) {
-                    this.service = (ClientService) source;
+                    this.service = (ClientService) state.source;
                 } else if (type.is_a(typeof(Folder))) {
-                    this.folder = (Folder) source;
+                    this.folder = (Folder) state.source;
                 }
             }
             this.filled = true;
@@ -277,33 +261,11 @@ public class Record {
             str.append(": ");
         }
 
-        // Use a compact format for well known ojects
-        if (this.account != null) {
-            str.append(this.account.information.id);
-            str.append_c('[');
-            str.append(this.account.information.service_provider.to_value());
-            if (this.service != null) {
-                str.append_c(':');
-                str.append(this.service.configuration.protocol.to_value());
-            }
-            str.append_c(']');
-            if (this.folder == null) {
-                str.append(": ");
-            }
-        } else if (this.service != null) {
-            str.append(this.service.configuration.protocol.to_value());
-            str.append(": ");
-        }
-        if (this.folder != null) {
-            str.append(this.folder.path.to_string());
-            str.append(": ");
-        }
-
-        // Append in reverse so leaf sources appears last
-        var sources = get_other_sources();
-        for (int i = sources.length - 1; i >= 0; i--) {
-            str.append(sources[i].to_string());
-            str.append_c(' ');
+        // Append in reverse so inner sources appear first
+        for (int i = this.states.length - 1; i >= 0; i--) {
+            str.append("[");
+            str.append(this.states[i].format_message());
+            str.append("] ");
         }
 
         str.append(message);
diff --git a/src/engine/imap-engine/imap-engine-account-operation.vala 
b/src/engine/imap-engine/imap-engine-account-operation.vala
index 38c4b047..3dc32d37 100644
--- a/src/engine/imap-engine/imap-engine-account-operation.vala
+++ b/src/engine/imap-engine/imap-engine-account-operation.vala
@@ -112,8 +112,8 @@ public abstract class Geary.ImapEngine.AccountOperation : BaseObject, Logging.So
     }
 
     /** {@inheritDoc} */
-    public virtual string to_string() {
-        return get_type().name();
+    public virtual Logging.State to_logging_state() {
+        return new Logging.State(this, this.account.information.id);
     }
 
 }
@@ -160,14 +160,14 @@ public abstract class Geary.ImapEngine.FolderOperation : AccountOperation {
         );
     }
 
-    /**
-     * Provides a representation of this operation for debugging.
-     *
-     * The return value will include its folder's path and the name of
-     * the class.
-     */
-    public override string to_string() {
-        return "%s(%s)".printf(base.to_string(), folder.path.to_string());
+    /** {@inheritDoc} */
+    public override Logging.State to_logging_state() {
+        return new Logging.State(
+            this,
+            "%s:%s",
+            this.account.information.id,
+            this.folder.path.to_string()
+        );
     }
 
 }
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 06ae06f5..37694c47 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1411,9 +1411,14 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         yield op.wait_for_ready_async(cancellable);
     }
 
-    public override string to_string() {
-        return "%s (open_count=%d remote_opened=%s)".printf(
-            base.to_string(), open_count, (this.remote_session != null).to_string()
+    /** {@inheritDoc} */
+    public override Logging.State to_logging_state() {
+        return new Logging.State(
+            this,
+            "%s, open_count=%d, remote_opened=%s",
+            this.path.to_string(),
+            this.open_count,
+            (this.remote_session != null).to_string()
         );
     }
 
diff --git a/src/engine/util/util-logging.vala b/src/engine/util/util-logging.vala
index 3fac688d..30ff8ffe 100644
--- a/src/engine/util/util-logging.vala
+++ b/src/engine/util/util-logging.vala
@@ -5,6 +5,7 @@
  * (version 2.1 or later). See the COPYING file in this distribution.
  */
 
+
 /**
  * Mixin interface for objects that support structured logging.
  *
@@ -18,6 +19,22 @@
 public interface Geary.Logging.Source : GLib.Object {
 
 
+    /**
+     * Returns a string representation of a source based on its state.
+     *
+     * The string returned will include the source's type name, the
+     * its current logging state, and the value of extra_values, if
+     * any.
+     */
+    protected static string default_to_string(Source source,
+                                              string extra_values) {
+        return "%s(%s%s)".printf(
+            source.get_type().name(),
+            source.to_logging_state().format_message(),
+            extra_values
+        );
+    }
+
     // Based on function from with the same name from GLib's
     // gmessages.c. Return value must be 1 byte long (plus nul byte).
     // Reference:
@@ -114,10 +131,26 @@ public interface Geary.Logging.Source : GLib.Object {
     public abstract Source? logging_parent { get; }
 
     /**
-     * Returns a string representation of the service, for debugging.
+     * Returns a loggable representation of this source's current state.
+     *
+     * Since this source's internal state may change between being
+     * logged and being used from a log record, this records relevant
+     * state at the time when it was logged so it may be displayed or
+     * recorded as it is right now.
      */
-    public abstract string to_string();
+    public abstract State to_logging_state();
 
+    /**
+     * Returns a string representation of this source based on its state.
+     *
+     * This simply calls {@link default_to_string} with this source
+     * and the empty string, returning the result. Implementations of
+     * this interface can call that method if they need to override
+     * the default behaviour of this method.
+     */
+    public string to_string() {
+        return Source.default_to_string(this, "");
+    }
 
     /**
      * Logs a debug-level log message with this object as context.
@@ -185,3 +218,45 @@ public interface Geary.Logging.Source : GLib.Object {
     }
 
 }
+
+/**
+ * A record of the state of a logging source to be recorded.
+ *
+ * @see Source.to_logging_state
+ */
+// This a class rather than a struct so we get pass-by-reference
+// semantics for it, and make its members private
+public class Geary.Logging.State {
+
+
+    public Source source { get; private set; }
+
+
+    private string message;
+    // Would like to use the following but can't because of
+    // https://gitlab.gnome.org/GNOME/vala/issues/884
+    // private va_list args;
+
+
+    /*
+     * Constructs a new logging state.
+     *
+     * The given source should be the source object that constructed
+     * the state during a call to {@link Source.to_logging_state}.
+     */
+    [PrintfFormat]
+    public State(Source source, string message, ...) {
+        this.source = source;
+        this.message = message;
+
+        // this.args = va_list();
+        this.message = message.vprintf(va_list());
+    }
+
+    public string format_message() {
+        // vprint mangles its passed-in args, so copy them
+        // return this.message.vprintf(va_list.copy(this.args));
+        return message;
+    }
+
+}