From cb16bdc59d2abfad02f944970603594220e5bdfd Mon Sep 17 00:00:00 2001
From: Michael Gratton <mike@vee.net>
Date: Tue, 10 Dec 2019 13:18:45 +1100
Subject: [PATCH] Remove default Geary.EmailIdentifier::hash and ::equal_to
 impls

Make subclasses implement these themselves and remove the unique string
property, to be (hopefully) more efficient and easier for subclasses
to specialise.
---
 src/engine/api/geary-email-identifier.vala    | 28 ++++++-------------
 .../imap-db/imap-db-email-identifier.vala     | 25 ++++++++++++-----
 .../outbox/outbox-email-identifier.vala       | 21 +++++++++++++-
 .../api/geary-email-identifier-mock.vala      | 28 +++++++++++++++----
 4 files changed, 69 insertions(+), 33 deletions(-)

diff --git a/src/engine/api/geary-email-identifier.vala b/src/engine/api/geary-email-identifier.vala
index cdea6a7b..3dda2f83 100644
--- a/src/engine/api/geary-email-identifier.vala
+++ b/src/engine/api/geary-email-identifier.vala
@@ -18,21 +18,18 @@
  * they will be unique throughout the Geary engine.
  */
 
-public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable<Geary.EmailIdentifier> {
+public abstract class Geary.EmailIdentifier :
+    BaseObject, Gee.Hashable<Geary.EmailIdentifier> {
 
     /** Base variant type returned by {@link to_variant}. */
     public const string BASE_VARIANT_TYPE = "(yr)";
 
-    // Warning: only change this if you know what you are doing.
-    protected string unique;
 
-    protected EmailIdentifier(string unique) {
-        this.unique = unique;
-    }
+    /** {@inheritDoc} */
+    public abstract uint hash();
 
-    public virtual uint hash() {
-        return unique.hash();
-    }
+    /** {@inheritDoc} */
+    public abstract bool equal_to(EmailIdentifier other);
 
     /**
      * Returns a representation useful for serialisation.
@@ -50,16 +47,7 @@ public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable<Geary.Ema
     /**
      * Returns a representation useful for debugging.
      */
-    public virtual string to_string() {
-        return "[%s]".printf(unique.to_string());
-    }
-
-    public virtual bool equal_to(Geary.EmailIdentifier other) {
-        if (this == other)
-            return true;
-
-        return unique == other.unique;
-    }
+    public abstract string to_string();
 
     /**
      * A comparator for stabilizing sorts.
@@ -71,7 +59,7 @@ public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable<Geary.Ema
         if (this == other)
             return 0;
 
-        return strcmp(unique, other.unique);
+        return strcmp(to_string(), other.to_string());
     }
 
     /**
diff --git a/src/engine/imap-db/imap-db-email-identifier.vala b/src/engine/imap-db/imap-db-email-identifier.vala
index cb2c5dc1..3a0951b0 100644
--- a/src/engine/imap-db/imap-db-email-identifier.vala
+++ b/src/engine/imap-db/imap-db-email-identifier.vala
@@ -18,8 +18,6 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier {
     public EmailIdentifier(int64 message_id, Imap.UID? uid) {
         assert(message_id != Db.INVALID_ROWID);
 
-        base (message_id.to_string());
-
         this.message_id = message_id;
         this.uid = uid;
     }
@@ -27,8 +25,6 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier {
     // Used when a new message comes off the wire and doesn't have a rowid associated with it (yet)
     // Requires a UID in order to find or create such an association
     public EmailIdentifier.no_message_id(Imap.UID uid) {
-        base (Db.INVALID_ROWID.to_string());
-
         message_id = Db.INVALID_ROWID;
         this.uid = uid;
     }
@@ -56,8 +52,6 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier {
     // you not to be able to find them.
     public void promote_with_message_id(int64 message_id) {
         assert(this.message_id == Db.INVALID_ROWID);
-
-        unique = message_id.to_string();
         this.message_id = message_id;
     }
 
@@ -65,6 +59,19 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier {
         return (uid != null) && uid.is_valid();
     }
 
+    /** {@inheritDoc} */
+    public override uint hash() {
+        return GLib.int64_hash(this.message_id);
+    }
+
+    /** {@inheritDoc} */
+    public override bool equal_to(Geary.EmailIdentifier other) {
+        return (
+            this.get_type() == other.get_type() &&
+            this.message_id == ((EmailIdentifier) other).message_id
+        );
+    }
+
     public override int natural_sort_comparator(Geary.EmailIdentifier o) {
         ImapDB.EmailIdentifier? other = o as ImapDB.EmailIdentifier;
         if (other == null)
@@ -93,7 +100,11 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier {
     }
 
     public override string to_string() {
-        return "[%s/%s]".printf(message_id.to_string(), (uid == null ? "null" : uid.to_string()));
+        return "%s(%lld,%s)".printf(
+            this.get_type().name(),
+            this.message_id,
+            this.uid != null ? this.uid.to_string() : "null"
+        );
     }
 
     public static Gee.Set<Imap.UID> to_uids(Gee.Collection<ImapDB.EmailIdentifier> ids) {
diff --git a/src/engine/outbox/outbox-email-identifier.vala b/src/engine/outbox/outbox-email-identifier.vala
index 63b6f67c..76aa34f3 100644
--- a/src/engine/outbox/outbox-email-identifier.vala
+++ b/src/engine/outbox/outbox-email-identifier.vala
@@ -16,7 +16,6 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier {
 
 
     public EmailIdentifier(int64 message_id, int64 ordering) {
-        base("Outbox.EmailIdentifier:%s".printf(message_id.to_string()));
         this.message_id = message_id;
         this.ordering = ordering;
     }
@@ -34,6 +33,26 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier {
         this(mid.get_int64(), ord.get_int64());
     }
 
+    /** {@inheritDoc} */
+    public override uint hash() {
+        return GLib.int64_hash(this.message_id);
+    }
+
+    /** {@inheritDoc} */
+    public override bool equal_to(Geary.EmailIdentifier other) {
+        return (
+            this.get_type() == other.get_type() &&
+            this.message_id == ((EmailIdentifier) other).message_id
+        );
+    }
+
+    /** {@inheritDoc} */
+    public override string to_string() {
+        return "%s(%lld,%lld)".printf(
+            this.get_type().name(), this.message_id, this.ordering
+        );
+    }
+
     public override int natural_sort_comparator(Geary.EmailIdentifier o) {
         EmailIdentifier? other = o as EmailIdentifier;
         if (other == null) {
diff --git a/test/engine/api/geary-email-identifier-mock.vala b/test/engine/api/geary-email-identifier-mock.vala
index 2928ca16..61b56bdc 100644
--- a/test/engine/api/geary-email-identifier-mock.vala
+++ b/test/engine/api/geary-email-identifier-mock.vala
@@ -12,17 +12,35 @@ public class Geary.MockEmailIdentifer : EmailIdentifier {
 
 
     public MockEmailIdentifer(int id) {
-        base(id.to_string());
         this.id = id;
     }
 
+    public override uint hash() {
+        return GLib.int_hash(this.id);
+    }
+
+    public override bool equal_to(Geary.EmailIdentifier other) {
+        return (
+            this.get_type() == other.get_type() &&
+            this.id == ((MockEmailIdentifer) other).id
+        );
+    }
+
+
+    public override string to_string() {
+        return "%s(%d)".printf(
+            this.get_type().name(),
+            this.id
+        );
+    }
+
+    public override GLib.Variant to_variant() {
+        return new GLib.Variant.int32(id);
+    }
+
     public override int natural_sort_comparator(Geary.EmailIdentifier other) {
         MockEmailIdentifer? other_mock = other as MockEmailIdentifer;
         return (other_mock == null) ? 1 : this.id - other_mock.id;
     }
 
-    public override GLib.Variant to_variant() {
-        return new GLib.Variant.int32(id);
-    }
-
 }