From 5e13dd0abdc440763a8cb692542e510e1ca5cece Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 11 Jun 2019 07:21:35 +1000 Subject: [PATCH] Replace Db.PrepareDelegate with a subclass virtual function hook While slightly less flexible, it fits the Engine's needs and simplifies the DB API. --- src/engine/db/db-database.vala | 19 ++++++++++++------- src/engine/db/db-versioned-database.vala | 7 +++---- src/engine/db/db.vala | 14 -------------- src/engine/imap-db/imap-db-database.vala | 5 +++-- test/engine/db/db-database-test.vala | 14 +++++++------- .../engine/db/db-versioned-database-test.vala | 2 +- .../imap-db/imap-db-attachment-test.vala | 2 +- 7 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/engine/db/db-database.vala b/src/engine/db/db-database.vala index 404aa4f0..e28180a4 100644 --- a/src/engine/db/db-database.vala +++ b/src/engine/db/db-database.vala @@ -65,7 +65,6 @@ public class Geary.Db.Database : Geary.Db.Context { private Connection? primary = null; private int outstanding_async_jobs = 0; private ThreadPool? thread_pool = null; - private unowned PrepareConnection? prepare_cb = null; /** * Constructs a new database that is persisted on disk. @@ -104,14 +103,12 @@ public class Geary.Db.Database : Geary.Db.Context { * and Results. */ public virtual async void open(DatabaseFlags flags, - PrepareConnection? prepare_cb, Cancellable? cancellable = null) throws Error { if (is_open) return; this.flags = flags; - this.prepare_cb = prepare_cb; if (this.file != null && (flags & DatabaseFlags.CREATE_DIRECTORY) != 0) { yield Geary.Files.make_directory_with_parents(this.file.get_parent()); @@ -235,9 +232,7 @@ public class Geary.Db.Database : Geary.Db.Context { } Connection cx = new Connection(this, sqlite_flags, cancellable); - if (prepare_cb != null) - prepare_cb(cx, is_primary); - + prepare_connection(cx); return cx; } @@ -365,6 +360,17 @@ public class Geary.Db.Database : Geary.Db.Context { this.thread_pool.add(new_job); } + /** + * Hook for subclasses to modify a new SQLite connection before use. + * + * This allows sub-classes to configure SQLite on a newly + * established connections before being used, such as setting + * pragmas, custom collation functions, and so on, + */ + protected virtual void prepare_connection(Connection cx) throws GLib.Error { + // No-op by default; + } + // This method must be thread-safe. private void on_async_job(owned TransactionAsyncJob job) { // *never* use primary connection for threaded operations @@ -395,4 +401,3 @@ public class Geary.Db.Database : Geary.Db.Context { return this; } } - diff --git a/src/engine/db/db-versioned-database.vala b/src/engine/db/db-versioned-database.vala index 24b2403c..ae9717a1 100644 --- a/src/engine/db/db-versioned-database.vala +++ b/src/engine/db/db-versioned-database.vala @@ -86,10 +86,9 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database { * the application. */ public override async void open(DatabaseFlags flags, - PrepareConnection? prepare_cb, - Cancellable? cancellable = null) - throws Error { - yield base.open(flags, prepare_cb, cancellable); + GLib.Cancellable? cancellable = null) + throws GLib.Error { + yield base.open(flags, cancellable); // get Connection for upgrade activity Connection cx = yield open_connection(cancellable); diff --git a/src/engine/db/db.vala b/src/engine/db/db.vala index 9b4217df..ff362496 100644 --- a/src/engine/db/db.vala +++ b/src/engine/db/db.vala @@ -45,20 +45,6 @@ public enum ResetScope { CLEAR_BINDINGS } -/* - * PrepareConnection is called from Database when a Connection is created. Database may pool - * Connections, especially for asynchronous queries, so this is only called when a new - * Connection is created and not when its reused. - * - * PrepareConnection may be used as an opportunity to modify or configure the Connection. - * This callback is called prior to the Connection being used, either internally or handed off to - * a caller for normal use. - * - * This callback may be called in the context of a background thread. - */ -public delegate void PrepareConnection(Connection cx, bool is_primary) - throws GLib.Error; - /** * See Connection.exec_transaction() for more information on how this delegate is used. */ diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala index 8c4e4761..f8a4b588 100644 --- a/src/engine/imap-db/imap-db-database.vala +++ b/src/engine/imap-db/imap-db-database.vala @@ -36,7 +36,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { */ public new async void open(Db.DatabaseFlags flags, Cancellable? cancellable) throws Error { - yield base.open(flags, on_prepare_database_connection, cancellable); + yield base.open(flags, cancellable); // Tie user-supplied Cancellable to internal Cancellable, which is used when close() is // called @@ -579,7 +579,8 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { stmt.exec(); } - private void on_prepare_database_connection(Db.Connection cx) throws Error { + protected override void prepare_connection(Db.Connection cx) + throws GLib.Error { cx.set_busy_timeout_msec(Db.Connection.RECOMMENDED_BUSY_TIMEOUT_MSEC); cx.set_foreign_keys(true); cx.set_recursive_triggers(true); diff --git a/test/engine/db/db-database-test.vala b/test/engine/db/db-database-test.vala index 93502489..c8e746bd 100644 --- a/test/engine/db/db-database-test.vala +++ b/test/engine/db/db-database-test.vala @@ -22,7 +22,7 @@ class Geary.Db.DatabaseTest : TestCase { public void transient_open() throws Error { Database db = new Geary.Db.Database.transient(); db.open.begin( - Geary.Db.DatabaseFlags.NONE, null, null, + Geary.Db.DatabaseFlags.NONE, null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); @@ -40,7 +40,7 @@ class Geary.Db.DatabaseTest : TestCase { Database db = new Geary.Db.Database.persistent(tmp_file); db.open.begin( - Geary.Db.DatabaseFlags.NONE, null, null, + Geary.Db.DatabaseFlags.NONE, null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); @@ -61,7 +61,7 @@ class Geary.Db.DatabaseTest : TestCase { tmp_dir.get_child("test.db") ); db.open.begin( - Geary.Db.DatabaseFlags.CREATE_FILE, null, null, + Geary.Db.DatabaseFlags.CREATE_FILE, null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); @@ -85,7 +85,7 @@ class Geary.Db.DatabaseTest : TestCase { db.open.begin( Geary.Db.DatabaseFlags.CREATE_DIRECTORY | Geary.Db.DatabaseFlags.CREATE_FILE, - null, null, + null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); @@ -110,7 +110,7 @@ class Geary.Db.DatabaseTest : TestCase { db.open.begin( Geary.Db.DatabaseFlags.CREATE_DIRECTORY | Geary.Db.DatabaseFlags.CREATE_FILE, - null, null, + null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); @@ -134,7 +134,7 @@ class Geary.Db.DatabaseTest : TestCase { db.open.begin( Geary.Db.DatabaseFlags.CREATE_FILE | Geary.Db.DatabaseFlags.CHECK_CORRUPTION, - null, null, + null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); @@ -159,7 +159,7 @@ class Geary.Db.DatabaseTest : TestCase { Geary.Db.DatabaseFlags.CREATE_DIRECTORY | Geary.Db.DatabaseFlags.CREATE_FILE | Geary.Db.DatabaseFlags.CHECK_CORRUPTION, - null, null, + null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); diff --git a/test/engine/db/db-versioned-database-test.vala b/test/engine/db/db-versioned-database-test.vala index 6d52abc8..4e12962f 100644 --- a/test/engine/db/db-versioned-database-test.vala +++ b/test/engine/db/db-versioned-database-test.vala @@ -34,7 +34,7 @@ class Geary.Db.VersionedDatabaseTest : TestCase { ); db.open.begin( - Geary.Db.DatabaseFlags.CREATE_FILE, null, null, + Geary.Db.DatabaseFlags.CREATE_FILE, null, (obj, ret) => { async_complete(ret); } ); db.open.end(async_result()); diff --git a/test/engine/imap-db/imap-db-attachment-test.vala b/test/engine/imap-db/imap-db-attachment-test.vala index 9456e6be..d24f2b8d 100644 --- a/test/engine/imap-db/imap-db-attachment-test.vala +++ b/test/engine/imap-db/imap-db-attachment-test.vala @@ -114,7 +114,7 @@ class Geary.ImapDB.AttachmentIoTest : TestCase { this.db = new Geary.Db.Database.transient(); this.db.open.begin( - Geary.Db.DatabaseFlags.NONE, null, null, + Geary.Db.DatabaseFlags.NONE, null, (obj, res) => { async_complete(res); } ); this.db.open.end(async_result());