[geary/wip/726281-text-attachment-crlf: 1/13] Support in-memory SQLite databases, as well as on-disk databases.

[Michael Gratton <mjog src gnome org>]

commit 4251596d0048f0d9e79cbc7f6593b3287d62cc9f
Author: Michael James Gratton <mike vee net>
Date:   Fri Apr 27 23:56:01 2018 +1000

    Support in-memory SQLite databases, as well as on-disk databases.
    
    Give Db.Database two constructors, one for persistent and one for
    transient databases. Add a path property since SQLite's API lives in the
    last century still and that gets used to specify in-memory databases, and
    use that for debug messages everwhere.
    
    Having support for in-memory databases is also needed to make unit
    testing ImapDB possible.

 src/engine/db/db-connection.vala         |   13 ++--
 src/engine/db/db-database.vala           |  103 ++++++++++++++++++++---------
 src/engine/db/db-versioned-database.vala |   26 +++++---
 src/engine/db/db.vala                    |    7 ++-
 src/engine/imap-db/imap-db-database.vala |   24 ++++---
 src/engine/imap-db/imap-db-folder.vala   |    8 +-
 src/engine/imap-db/imap-db-gc.vala       |   11 ++--
 7 files changed, 122 insertions(+), 70 deletions(-)
---
diff --git a/src/engine/db/db-connection.vala b/src/engine/db/db-connection.vala
index 96c6495..c9390cb 100644
--- a/src/engine/db/db-connection.vala
+++ b/src/engine/db/db-connection.vala
@@ -76,10 +76,12 @@ public class Geary.Db.Connection : Geary.Db.Context {
         }
         
         check_cancelled("Connection.ctor", cancellable);
-        
+
         try {
-            throw_on_error("Connection.ctor", Sqlite.Database.open_v2(database.db_file.get_path(),
-                out db, sqlite_flags, null));
+            throw_on_error(
+                "Connection.ctor",
+                Sqlite.Database.open_v2(database.path, out db, sqlite_flags, null)
+            );
         } catch (DatabaseError derr) {
             // don't throw BUSY error for open unless no db object was returned, as it's possible for
             // open_v2() to return an error *and* a valid Database object, see:
@@ -417,9 +419,8 @@ public class Geary.Db.Connection : Geary.Db.Context {
     public override Connection? get_connection() {
         return this;
     }
-    
+
     public string to_string() {
-        return "[%d] %s".printf(cx_number, database.db_file.get_basename());
+        return "[%d] %s".printf(cx_number, database.path);
     }
 }
-
diff --git a/src/engine/db/db-database.vala b/src/engine/db/db-database.vala
index b3989d4..7312032 100644
--- a/src/engine/db/db-database.vala
+++ b/src/engine/db/db-database.vala
@@ -17,11 +17,32 @@
  */
 
 public class Geary.Db.Database : Geary.Db.Context {
+
+
+    /** The path passed to SQLite to open a transient database. */
+    public const string MEMORY_PATH = "file::memory:?cache=shared";
+
+    /** The default number of threaded connections opened. */
     public const int DEFAULT_MAX_CONCURRENCY = 4;
-    
-    public File db_file { get; private set; }
+
+    /**
+     * The database's location on the filesystem.
+     *
+     * If null, this is a transient, in-memory database.
+     */
+    public File? file { get; private set; }
+
+    /**
+     * The path passed to Sqlite when opening the database.
+     *
+     * This will be the path to the database file on disk for
+     * persistent databases, else {@link MEMORY_PATH} for transient
+     * databases.
+     */
+    public string path { get; private set; }
+
     public DatabaseFlags flags { get; private set; }
-    
+
     private bool _is_open = false;
     public bool is_open {
         get {
@@ -36,16 +57,28 @@ public class Geary.Db.Database : Geary.Db.Context {
             }
         }
     }
-    
+
     private Connection? master_connection = null;
     private int outstanding_async_jobs = 0;
     private ThreadPool<TransactionAsyncJob>? thread_pool = null;
     private unowned PrepareConnection? prepare_cb = null;
-    
-    public Database(File db_file) {
-        this.db_file = db_file;
+
+    /**
+     * Constructs a new database that is persisted on disk.
+     */
+    public Database.persistent(File db_file) {
+        this.file = db_file;
+        this.path = db_file.get_path();
     }
-    
+
+    /**
+     * Constructs a new database that is stored in memory only.
+     */
+    public Database.transient() {
+        this.file = null;
+        this.path = MEMORY_PATH;
+    }
+
     ~Database() {
         // Not thrilled about using lock in a dtor
         lock (outstanding_async_jobs) {
@@ -68,13 +101,13 @@ public class Geary.Db.Database : Geary.Db.Context {
         
         this.flags = flags;
         this.prepare_cb = prepare_cb;
-        
-        if ((flags & DatabaseFlags.CREATE_DIRECTORY) != 0) {
-            File db_dir = db_file.get_parent();
+
+        if (this.file != null && (flags & DatabaseFlags.CREATE_DIRECTORY) != 0) {
+            File db_dir = this.file.get_parent();
             if (!db_dir.query_exists(cancellable))
                 db_dir.make_directory_with_parents(cancellable);
         }
-        
+
         if (threadsafe()) {
             if (thread_pool == null) {
                 thread_pool = new ThreadPool<TransactionAsyncJob>.with_owned_data(on_async_job,
@@ -91,7 +124,7 @@ public class Geary.Db.Database : Geary.Db.Context {
     }
     
     private void check_for_corruption(DatabaseFlags flags, Cancellable? cancellable) throws Error {
-        // if the file exists, open a connection and test for corruption by creating a dummy table,
+        // Open a connection and test for corruption by creating a dummy table,
         // adding a row, selecting the row, then dropping the table ... can only do this for
         // read-write databases, however
         //
@@ -100,8 +133,7 @@ public class Geary.Db.Database : Geary.Db.Context {
         //
         // TODO: Allow the caller to specify the name of the test table, so we're not clobbering
         // theirs (however improbable it is to name a table "CorruptionCheckTable")
-        bool exists = db_file.query_exists(cancellable);
-        if (exists && (flags & DatabaseFlags.READ_ONLY) == 0) {
+        if ((flags & DatabaseFlags.READ_ONLY) == 0) {
             Connection cx = new Connection(this, Sqlite.OPEN_READWRITE, cancellable);
             
             try {
@@ -120,17 +152,15 @@ public class Geary.Db.Database : Geary.Db.Context {
                 // drop table
                 cx.exec("DROP TABLE CorruptionCheckTable");
             } catch (Error err) {
-                throw new DatabaseError.CORRUPT("Possible integrity problem discovered in %s: %s",
-                    db_file.get_path(), err.message);
+                throw new DatabaseError.CORRUPT(
+                    "Possible integrity problem discovered in %s: %s",
+                    this.path,
+                    err.message
+                );
             }
-        } else if (!exists && (flags & DatabaseFlags.CREATE_FILE) == 0) {
-            // file doesn't exist and no flag to create it ... that's bad too, might as well
-            // let them know now
-            throw new DatabaseError.CORRUPT("Database file %s not found and no CREATE_FILE flag",
-                db_file.get_path());
         }
     }
-    
+
     /**
      * Closes the Database, releasing any resources it may hold, including the master connection.
      *
@@ -151,12 +181,15 @@ public class Geary.Db.Database : Geary.Db.Context {
         
         is_open = false;
     }
-    
+
     private void check_open() throws Error {
-        if (!is_open)
-            throw new DatabaseError.OPEN_REQUIRED("Database %s not open", db_file.get_path());
+        if (!is_open) {
+            throw new DatabaseError.OPEN_REQUIRED(
+                "Database %s not open", this.path
+            );
+        }
     }
-    
+
     /**
      * Throws DatabaseError.OPEN_REQUIRED if not open.
      */
@@ -166,12 +199,18 @@ public class Geary.Db.Database : Geary.Db.Context {
     
     private Connection internal_open_connection(bool master, Cancellable? cancellable) throws Error {
         check_open();
-        
-        int sqlite_flags = (flags & DatabaseFlags.READ_ONLY) != 0 ? Sqlite.OPEN_READONLY
+
+        int sqlite_flags = (flags & DatabaseFlags.READ_ONLY) != 0
+            ? Sqlite.OPEN_READONLY
             : Sqlite.OPEN_READWRITE;
+
         if ((flags & DatabaseFlags.CREATE_FILE) != 0)
             sqlite_flags |= Sqlite.OPEN_CREATE;
-        
+
+        if (this.file == null) {
+            sqlite_flags |= SQLITE_OPEN_URI;
+        }
+
         Connection cx = new Connection(this, sqlite_flags, cancellable);
         if (prepare_cb != null)
             prepare_cb(cx, master);
@@ -276,9 +315,9 @@ public class Geary.Db.Database : Geary.Db.Context {
         } catch (Error err) {
             open_err = err;
             debug("Warning: unable to open database connection to %s, cancelling AsyncJob: %s",
-                db_file.get_path(), err.message);
+                  this.path, err.message);
         }
-        
+
         if (cx != null)
             job.execute(cx);
         else
diff --git a/src/engine/db/db-versioned-database.vala b/src/engine/db/db-versioned-database.vala
index 71c5b54..e4a8b57 100644
--- a/src/engine/db/db-versioned-database.vala
+++ b/src/engine/db/db-versioned-database.vala
@@ -10,13 +10,19 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database {
     private static Mutex upgrade_mutex = Mutex();
 
     public File schema_dir { get; private set; }
-    
-    public VersionedDatabase(File db_file, File schema_dir) {
-        base (db_file);
-        
+
+    /** {@inheritDoc} */
+    public VersionedDatabase.persistent(File db_file, File schema_dir) {
+        base.persistent(db_file);
         this.schema_dir = schema_dir;
     }
-    
+
+    /** {@inheritDoc} */
+    public VersionedDatabase.transient(File schema_dir) {
+        base.transient();
+        this.schema_dir = schema_dir;
+    }
+
     /**
      * Called by {@link open} if a schema upgrade is required and beginning.
      *
@@ -74,9 +80,9 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database {
         Connection cx = open_connection(cancellable);
         
         int db_version = cx.get_user_version_number();
-        debug("VersionedDatabase.upgrade: current database schema for %s: %d", db_file.get_path(),
-            db_version);
-        
+        debug("VersionedDatabase.upgrade: current database schema for %s: %d",
+              this.path, db_version);
+
         // If the DB doesn't exist yet, the version number will be zero, but also treat negative
         // values as new
         bool new_db = db_version <= 0;
@@ -95,9 +101,9 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database {
         // *next* version of the database
         if (db_version > 0 && !get_schema_file(db_version).query_exists(cancellable)) {
             throw new DatabaseError.SCHEMA_VERSION("%s schema %d unknown to current schema plan",
-                db_file.get_path(), db_version);
+                this.path, db_version);
         }
-        
+
         // Go through all the version scripts in the schema directory and apply each of them.
         bool started = false;
         for (;;) {
diff --git a/src/engine/db/db.vala b/src/engine/db/db.vala
index 5b04b1e..c3f60a5 100644
--- a/src/engine/db/db.vala
+++ b/src/engine/db/db.vala
@@ -22,6 +22,9 @@
  * [[http://code.google.com/p/sqlheavy/|SQLHeavy]].
  */
 
+// Work around missing const in sqlite3.vapi. See Bug 795627.
+extern const int SQLITE_OPEN_URI;
+
 extern int sqlite3_enable_shared_cache(int enabled);
 
 namespace Geary.Db {
@@ -107,8 +110,8 @@ private int throw_on_error(Context ctx, string? method, int result, string? raw
     }
     
     string location = !String.is_empty(method)
-        ? "(%s %s) ".printf(method, ctx.get_database().db_file.get_path())
-        : "(%s) ".printf(ctx.get_database().db_file.get_path());
+        ? "(%s %s) ".printf(method, ctx.get_database().path)
+        : "(%s) ".printf(ctx.get_database().path);
     string errmsg = (ctx.get_connection() != null) ? " - %s".printf(ctx.get_connection().db.errmsg()) : "";
     string sql;
     if (ctx.get_statement() != null)
diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala
index 11e6058..61c7510 100644
--- a/src/engine/imap-db/imap-db-database.vala
+++ b/src/engine/imap-db/imap-db-database.vala
@@ -15,11 +15,13 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
     private string account_owner_email;
     private bool new_db = false;
     private Cancellable gc_cancellable = new Cancellable();
-    
-    public Database(File db_dir, File schema_dir, ProgressMonitor upgrade_monitor,
-        ProgressMonitor vacuum_monitor, string account_owner_email) {
-        base (get_db_file(db_dir), schema_dir);
-        
+
+    public Database(GLib.File db_dir,
+                    GLib.File schema_dir,
+                    ProgressMonitor upgrade_monitor,
+                    ProgressMonitor vacuum_monitor,
+                    string account_owner_email) {
+        base.persistent(get_db_file(db_dir), schema_dir);
         this.upgrade_monitor = upgrade_monitor;
         this.vacuum_monitor = vacuum_monitor;
 
@@ -61,8 +63,9 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
             try {
                 yield gc.vacuum_async(gc_cancellable);
             } catch (Error err) {
-                message("Vacuum of IMAP database %s failed: %s", db_file.get_path(), err.message);
-                
+                message(
+                    "Vacuum of IMAP database %s failed: %s", this.path, err.message
+                );
                 throw err;
             } finally {
                 if (vacuum_monitor.is_in_progress)
@@ -85,7 +88,8 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
         try {
             gc.reap_async.end(result);
         } catch (Error err) {
-            message("Garbage collection of IMAP database %s failed: %s", db_file.get_path(), err.message);
+            message("Garbage collection of IMAP database %s failed: %s",
+                    this.path, err.message);
         }
     }
     
@@ -413,7 +417,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
                         
                         debug("Invalid INTERNALDATE \"%s\" found at row %s in %s: %s",
                             internaldate != null ? internaldate : "(null)",
-                            invalid_id.to_string(), db_file.get_path(), err.message);
+                            invalid_id.to_string(), this.path, err.message);
                         invalid_ids.set(invalid_id, (Geary.Email.Field) results.int_at(2));
                     }
                     
@@ -444,7 +448,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
             });
         } catch (Error err) {
             debug("Error fixing INTERNALDATES during upgrade to schema 15 for %s: %s",
-                db_file.get_path(), err.message);
+                  this.path, err.message);
         }
     }
     
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index 8c236e1..0d4dcd8 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -2097,7 +2097,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
                     results.string_at(6),
                     disposition,
                     content_filename,
-                    cx.database.db_file.get_parent(),
+                    cx.database.file.get_parent(),
                     results.int64_at(3)
                 )
             );
@@ -2158,10 +2158,10 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
             stmt.bind_string(6, description);
             
             int64 attachment_id = stmt.exec_insert(cancellable);
-            
-            File saved_file = ImapDB.Attachment.generate_file(db.db_file.get_parent(), message_id,
+
+            File saved_file = ImapDB.Attachment.generate_file(db.file.get_parent(), message_id,
                 attachment_id, filename);
-            
+
             // On the off-chance this is marked for deletion, unmark it
             try {
                 stmt = cx.prepare("""
diff --git a/src/engine/imap-db/imap-db-gc.vala b/src/engine/imap-db/imap-db-gc.vala
index 8d07327..11ec3b1 100644
--- a/src/engine/imap-db/imap-db-gc.vala
+++ b/src/engine/imap-db/imap-db-gc.vala
@@ -88,7 +88,7 @@ private class Geary.ImapDB.GC {
     public GC(ImapDB.Database db, int priority) {
         this.db = db;
         this.priority = priority;
-        data_dir = db.db_file.get_parent();
+        data_dir = db.file.get_parent();
     }
 
     /**
@@ -571,8 +571,8 @@ private class Geary.ImapDB.GC {
     
     private async int delete_empty_attachment_directories_async(File? current, out bool empty,
         Cancellable? cancellable) throws Error {
-        File current_dir = current ?? Attachment.get_attachments_dir(db.db_file.get_parent());
-        
+        File current_dir = current ?? Attachment.get_attachments_dir(db.file.get_parent());
+
         // directory is considered empty until file or non-deleted child directory is found
         empty = true;
         
@@ -662,9 +662,8 @@ private class Geary.ImapDB.GC {
         reaped_messages_since_last_vacuum = reaped_count;
         free_page_bytes = free_page_count * page_size;
     }
-    
+
     public string to_string() {
-        return "GC:%s".printf(db.db_file.get_path());
+        return "GC:%s".printf(db.path);
     }
 }
-