From bf4f5c478092df4e99302b088a4369f0994597c0 Mon Sep 17 00:00:00 2001
From: Michael Gratton <mike@vee.net>
Date: Wed, 30 Oct 2019 14:12:52 +1100
Subject: [PATCH] Fix GenericAccount::to_email_identifier not actually working

The GVariant type "*" only matches a single data type, not many, and
the sense of the test to check serialised ids was wrong anyway. As a
reuslt, this method probably never worked. Add a unit test.
---
 src/engine/api/geary-email-identifier.vala    |  7 +-
 .../imap-engine-generic-account.vala          | 10 +-
 .../imap-engine-generic-account-test.vala     | 93 +++++++++++++++++++
 test/meson.build                              |  1 +
 test/test-engine.vala                         |  1 +
 5 files changed, 106 insertions(+), 6 deletions(-)
 create mode 100644 test/engine/imap-engine/imap-engine-generic-account-test.vala

diff --git a/src/engine/api/geary-email-identifier.vala b/src/engine/api/geary-email-identifier.vala
index 5976ac16..e1df28ed 100644
--- a/src/engine/api/geary-email-identifier.vala
+++ b/src/engine/api/geary-email-identifier.vala
@@ -19,6 +19,10 @@
  */
 
 public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable<Geary.EmailIdentifier> {
+
+    /** Base variant type returned by {@link to_variant}. */
+    public const string BASE_VARIANT_TYPE = "(y??)";
+
     // Warning: only change this if you know what you are doing.
     protected string unique;
 
@@ -37,7 +41,8 @@ public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable<Geary.Ema
      * Action parameters, and so on.
      *
      * @return a serialised form of this id, that will match the
-     * GVariantType `(*)`
+     * GVariantType given by {@link BASE_VARIANT_TYPE}.
+     *
      * @see Account.to_email_identifier
      */
     public abstract GLib.Variant to_variant();
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala
index c5ada125..c7a28c75 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -25,7 +25,9 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         Geary.SpecialFolderType.ARCHIVE,
     };
 
-    private static GLib.VariantType email_id_type = new GLib.VariantType("(y*)");
+    private static GLib.VariantType email_id_type = new GLib.VariantType(
+        EmailIdentifier.BASE_VARIANT_TYPE
+    );
 
 
     /** Service for incoming IMAP connections. */
@@ -408,10 +410,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     /** {@inheritDoc} */
     public override EmailIdentifier to_email_identifier(GLib.Variant serialised)
         throws EngineError.BAD_PARAMETERS {
-        if (serialised.is_of_type(GenericAccount.email_id_type)) {
-            throw new EngineError.BAD_PARAMETERS(
-                "Invalid outer serialised type: (y*)"
-            );
+        if (!serialised.is_of_type(GenericAccount.email_id_type)) {
+            throw new EngineError.BAD_PARAMETERS("Invalid outer serialised type");
         }
         char type = (char) serialised.get_child_value(0).get_byte();
         if (type == 'i')
diff --git a/test/engine/imap-engine/imap-engine-generic-account-test.vala b/test/engine/imap-engine/imap-engine-generic-account-test.vala
new file mode 100644
index 00000000..73f9793b
--- /dev/null
+++ b/test/engine/imap-engine/imap-engine-generic-account-test.vala
@@ -0,0 +1,93 @@
+/*
+ * Copyright 2019 Michael Gratton <mike@vee.net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+public class Geary.ImapEngine.GenericAccountTest : TestCase {
+
+
+    internal class TestAccount : GenericAccount {
+
+        public TestAccount(AccountInformation config,
+                           ImapDB.Account local,
+                           Endpoint incoming_remote,
+                           Endpoint outgoing_remote) {
+            base(config, local, incoming_remote, outgoing_remote);
+        }
+
+        protected override MinimalFolder new_folder(ImapDB.Folder local_folder) {
+            return new MinimalFolder(
+                this,
+                local_folder,
+                NONE
+            );
+        }
+
+    }
+
+
+    private GLib.File? tmp_dir = null;
+    private Geary.AccountInformation? config = null;
+    private ImapDB.Account? local_account = null;
+
+
+    public GenericAccountTest() {
+        base("Geary.ImapEngine.GenericAccountTest");
+        add_test("to_email_identifier", to_email_identifier);
+    }
+
+    public override void set_up() throws GLib.Error {
+        this.tmp_dir = GLib.File.new_for_path(
+            GLib.DirUtils.make_tmp(
+                "geary-imap-engine-generic-account-test-XXXXXX"
+            )
+        );
+
+        this.config = new Geary.AccountInformation(
+            "test",
+            ServiceProvider.OTHER,
+            new MockCredentialsMediator(),
+            new Geary.RFC822.MailboxAddress(null, "test@example.com")
+        );
+
+        this.local_account = new ImapDB.Account(
+            config,
+            this.tmp_dir,
+            GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql")
+        );
+        this.local_account.open_async.begin(
+            null,
+            (obj, ret) => { async_complete(ret); }
+        );
+        this.local_account.open_async.end(async_result());
+    }
+
+    public override void tear_down() throws GLib.Error {
+        this.local_account.close_async.begin(
+            null,
+            (obj, ret) => { async_complete(ret); }
+        );
+        this.local_account.close_async.end(async_result());
+        this.local_account = null;
+        this.config = null;
+
+        delete_file(this.tmp_dir);
+        this.tmp_dir = null;
+    }
+
+    public void to_email_identifier() throws GLib.Error {
+        TestAccount test_article = new TestAccount(
+            this.config,
+            this.local_account,
+            new Endpoint(new GLib.NetworkAddress("localhost", 143), NONE, 0),
+            new Endpoint(new GLib.NetworkAddress("localhost", 25), NONE, 0)
+        );
+
+        assert_non_null(
+            test_article.to_email_identifier(new GLib.Variant("(yxx)", 'i', 1, 2))
+        );
+    }
+
+}
diff --git a/test/meson.build b/test/meson.build
index e8f013fc..e774d7aa 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -47,6 +47,7 @@ geary_test_engine_sources = [
   'engine/imap-db/imap-db-email-identifier-test.vala',
   'engine/imap-db/imap-db-folder-test.vala',
   'engine/imap-engine/account-processor-test.vala',
+  'engine/imap-engine/imap-engine-generic-account-test.vala',
   'engine/mime-content-type-test.vala',
   'engine/outbox/outbox-email-identifier-test.vala',
   'engine/rfc822-mailbox-address-test.vala',
diff --git a/test/test-engine.vala b/test/test-engine.vala
index d8cc5c6d..28e758e1 100644
--- a/test/test-engine.vala
+++ b/test/test-engine.vala
@@ -61,6 +61,7 @@ int main(string[] args) {
     engine.add_suite(new Geary.ImapDB.EmailIdentifierTest().get_suite());
     engine.add_suite(new Geary.ImapDB.FolderTest().get_suite());
     engine.add_suite(new Geary.ImapEngine.AccountProcessorTest().get_suite());
+    engine.add_suite(new Geary.ImapEngine.GenericAccountTest().get_suite());
 
     // Depends on ImapDb.Database working correctly
     engine.add_suite(new Geary.ContactStoreImplTest().get_suite());