[geary/wip/composer-folks: 12/22] Clean up ImapDB.Account ctor/open API a bit

[Michael Gratton <mjog src gnome org>]

commit 66a664f98d6a889a6c403bd005080949a7e804ab
Author: Michael Gratton <mike vee net>
Date:   Wed Jun 12 07:31:51 2019 +1000

    Clean up ImapDB.Account ctor/open API a bit
    
    Move args from open_async to ctor and use these to determine and store
    DB file and attachment paths as properties. This allows constructing
    the DB instance up front and markig it as non-nullable, simplifies
    calling patterns and hence allows removing one use of the deprecated
    Engine singleton, and by moving in the local data deletion
    implementation from ImapEngine.GenericAccount, also allows removing a
    static helper method.

 src/engine/api/geary-engine.vala                   |  6 +-
 src/engine/imap-db/imap-db-account.vala            | 88 ++++++++++++++--------
 .../imap-engine/imap-engine-generic-account.vala   | 40 ++--------
 test/engine/imap-db/imap-db-account-test.vala      |  8 +-
 test/engine/imap-db/imap-db-folder-test.vala       |  8 +-
 5 files changed, 78 insertions(+), 72 deletions(-)
---
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index c0000010..7d8231e8 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -306,7 +306,11 @@ public class Geary.Engine : BaseObject {
         if (account_instances.has_key(config.id))
             return account_instances.get(config.id);
 
-        ImapDB.Account local = new ImapDB.Account(config);
+        ImapDB.Account local = new ImapDB.Account(
+            config,
+            config.data_dir,
+            this.resource_dir.get_child("sql")
+        );
         Endpoint incoming_remote = get_shared_endpoint(
             config.service_provider, config.incoming
         );
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index 18de3c10..7748b28d 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -44,14 +44,6 @@ private class Geary.ImapDB.Account : BaseObject {
     private const string DB_FILENAME = "geary.db";
     private const string ATTACHMENTS_DIR = "attachments";
 
-    /**
-     * Returns the on-disk paths used for storage by this account.
-     */
-    public static void get_imap_db_storage_locations(File user_data_dir, out File db_file,
-        out File attachments_dir) {
-        db_file = user_data_dir.get_child(DB_FILENAME);
-        attachments_dir = user_data_dir.get_child(ATTACHMENTS_DIR);
-    }
 
     private class FolderReference : Geary.SmartReference {
         public Geary.FolderPath path;
@@ -63,6 +55,7 @@ private class Geary.ImapDB.Account : BaseObject {
         }
     }
 
+
     // Maps of localised search operator names and values to their
     // internal forms
     private static Gee.HashMap<string, string> search_op_names =
@@ -97,10 +90,12 @@ private class Geary.ImapDB.Account : BaseObject {
         ProgressType.DB_VACUUM); }
 
     /** The backing database for the account. */
-    public ImapDB.Database? db { get; private set; default = null; }
+    public ImapDB.Database db { get; private set; }
 
     private string name;
     private AccountInformation account_information;
+    private GLib.File db_file;
+    private GLib.File attachments_dir;
     private Gee.HashMap<Geary.FolderPath, FolderReference> folder_refs =
         new Gee.HashMap<Geary.FolderPath, FolderReference>();
     private Cancellable? background_cancellable = null;
@@ -253,14 +248,21 @@ private class Geary.ImapDB.Account : BaseObject {
         search_op_is_values.set(SEARCH_OP_VALUE_UNREAD, SEARCH_OP_VALUE_UNREAD);
     }
 
-    public Account(AccountInformation config) {
+    public Account(AccountInformation config,
+                   GLib.File data_dir,
+                   GLib.File schema_dir) {
         this.account_information = config;
         this.name = config.id + ":db";
-    }
+        this.db_file = data_dir.get_child(DB_FILENAME);
+        this.attachments_dir = data_dir.get_child(ATTACHMENTS_DIR);
 
-    private void check_open() throws Error {
-        if (db == null)
-            throw new EngineError.OPEN_REQUIRED("Database not open");
+        this.db = new ImapDB.Database(
+            this.db_file,
+            schema_dir,
+            this.attachments_dir,
+            upgrade_monitor,
+            vacuum_monitor
+        );
     }
 
     private ImapDB.SearchQuery check_search_query(Geary.SearchQuery q) throws Error {
@@ -271,24 +273,11 @@ private class Geary.ImapDB.Account : BaseObject {
         return query;
     }
 
-    public async void open_async(File user_data_dir, File schema_dir, Cancellable? cancellable)
-        throws Error {
-        if (this.db != null)
+    public async void open_async(GLib.Cancellable? cancellable)
+        throws GLib.Error {
+        if (this.db.is_open) {
             throw new EngineError.ALREADY_OPEN("IMAP database already open");
-
-        File db_file;
-        File attachments_dir;
-        Account.get_imap_db_storage_locations(
-            user_data_dir, out db_file, out attachments_dir
-        );
-
-        this.db = new ImapDB.Database(
-            db_file,
-            schema_dir,
-            attachments_dir,
-            upgrade_monitor,
-            vacuum_monitor
-        );
+        }
 
         try {
             yield db.open(
@@ -299,7 +288,6 @@ private class Geary.ImapDB.Account : BaseObject {
 
             // close database before exiting
             db.close(null);
-            db = null;
 
             throw err;
         }
@@ -1886,5 +1874,39 @@ private class Geary.ImapDB.Account : BaseObject {
 
         return search_matches.size > 0 ? search_matches : null;
     }
-}
 
+    /** Removes database file and attachments directory. */
+    public async void delete_all_data(GLib.Cancellable? cancellable)
+        throws GLib.Error {
+        if (this.db.is_open) {
+            throw new EngineError.ALREADY_OPEN(
+                "Account cannot be open during rebuild"
+            );
+        }
+
+        if (yield Files.query_exists_async(this.db_file, cancellable)) {
+            message(
+                "%s: Deleting database file %s...",
+                this.name, this.db_file.get_path()
+            );
+            yield db_file.delete_async(GLib.Priority.DEFAULT, cancellable);
+        }
+
+        if (yield Files.query_exists_async(this.attachments_dir, cancellable)) {
+            message(
+                "%s: Deleting attachments directory %s...",
+                this.name, this.attachments_dir.get_path()
+            );
+            yield Files.recursive_delete_async(
+                this.attachments_dir, GLib.Priority.DEFAULT, cancellable
+            );
+        }
+    }
+
+    private inline void check_open() throws GLib.Error {
+        if (!this.db.is_open) {
+            throw new EngineError.OPEN_REQUIRED("Database not open");
+        }
+    }
+
+}
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 811dc6f6..ea303384 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -133,11 +133,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         this.processor.operation_error.connect(on_operation_error);
 
         try {
-            yield this.local.open_async(
-                information.data_dir,
-                Engine.instance.resource_dir.get_child("sql"),
-                cancellable
-            );
+            yield this.local.open_async(cancellable);
         } catch (Error err) {
             // convert database-open errors
             if (err is DatabaseError.CORRUPT)
@@ -245,36 +241,16 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         return open;
     }
 
-    public override async void rebuild_async(Cancellable? cancellable = null) throws Error {
-        if (open)
-            throw new EngineError.ALREADY_OPEN("Account cannot be open during rebuild");
-
-        message("%s: Rebuilding account local data", to_string());
-
-        // get all the storage locations associated with this Account
-        File db_file;
-        File attachments_dir;
-        ImapDB.Account.get_imap_db_storage_locations(information.data_dir, out db_file,
-            out attachments_dir);
-
-        if (yield Files.query_exists_async(db_file, cancellable)) {
-            message(
-                "%s: Deleting database file %s...",
-                to_string(), db_file.get_path()
-            );
-            yield db_file.delete_async(GLib.Priority.DEFAULT, cancellable);
-        }
-
-        if (yield Files.query_exists_async(attachments_dir, cancellable)) {
-            message(
-                "%s: Deleting attachments directory %s...",
-                to_string(), attachments_dir.get_path()
-            );
-            yield Files.recursive_delete_async(
-                attachments_dir, GLib.Priority.DEFAULT, cancellable
+    public override async void rebuild_async(GLib.Cancellable? cancellable = null)
+        throws GLib.Error {
+        if (this.open) {
+            throw new EngineError.ALREADY_OPEN(
+                "Account cannot be open during rebuild"
             );
         }
 
+        message("%s: Rebuilding account local data", to_string());
+        yield this.local.delete_all_data(cancellable);
         message("%s: Rebuild complete", to_string());
     }
 
diff --git a/test/engine/imap-db/imap-db-account-test.vala b/test/engine/imap-db/imap-db-account-test.vala
index 85b54c60..a82fb1ce 100644
--- a/test/engine/imap-db/imap-db-account-test.vala
+++ b/test/engine/imap-db/imap-db-account-test.vala
@@ -40,10 +40,12 @@ class Geary.ImapDB.AccountTest : TestCase {
             new Geary.RFC822.MailboxAddress(null, "test example com")
         );
 
-        this.account = new Account(config);
-        this.account.open_async.begin(
+        this.account = new Account(
+            config,
             this.tmp_dir,
-            GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql"),
+            GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql")
+        );
+        this.account.open_async.begin(
             null,
             (obj, ret) => { async_complete(ret); }
         );
diff --git a/test/engine/imap-db/imap-db-folder-test.vala b/test/engine/imap-db/imap-db-folder-test.vala
index df890003..0e5c6693 100644
--- a/test/engine/imap-db/imap-db-folder-test.vala
+++ b/test/engine/imap-db/imap-db-folder-test.vala
@@ -40,10 +40,12 @@ class Geary.ImapDB.FolderTest : TestCase {
             new Geary.RFC822.MailboxAddress(null, "test example com")
         );
 
-        this.account = new Account(config);
-        this.account.open_async.begin(
+        this.account = new Account(
+            config,
             this.tmp_dir,
-            GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql"),
+            GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql")
+        );
+        this.account.open_async.begin(
             null,
             (obj, ret) => { async_complete(ret); }
         );