[geary/wip/726281-text-attachment-crlf: 5/13] Push all ImapDB path management down into to ImapDb.Account.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/726281-text-attachment-crlf: 5/13] Push all ImapDB path management down into to ImapDb.Account.
- Date: Fri, 18 May 2018 23:27:27 +0000 (UTC)
commit 5c73fbcba5d06488ab86007ed366226bb2647170
Author: Michael James Gratton <mike vee net>
Date: Fri Apr 27 23:08:51 2018 +1000
Push all ImapDB path management down into to ImapDb.Account.
This stops generating account storage paths, and in particular the
location of attachment files, in a number of different places. Instead,
we determine the paths once, in ImapDb.Account and pass them around as
needed. This will help making the ImapDB classes unit-testable, and
in particular ImapDb.Folder by passing an instance of Db.Database,
rather than ImapDb.Database.
src/engine/imap-db/imap-db-account.vala | 102 +++++++++++++++---------
src/engine/imap-db/imap-db-attachment.vala | 8 +--
src/engine/imap-db/imap-db-database.vala | 40 ++++++---
src/engine/imap-db/imap-db-folder.vala | 88 ++++++++++++++-------
src/engine/imap-db/imap-db-gc.vala | 16 ++--
test/engine/imap-db/imap-db-database-test.vala | 3 +-
6 files changed, 163 insertions(+), 94 deletions(-)
---
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index a91acd0..b142dab 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -38,6 +38,20 @@ private class Geary.ImapDB.Account : BaseObject {
private const string SEARCH_OP_VALUE_STARRED = "starred";
private const string SEARCH_OP_VALUE_UNREAD = "unread";
+ // Storage path names
+ 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;
@@ -248,21 +262,27 @@ private class Geary.ImapDB.Account : BaseObject {
return query;
}
-
- public static void get_imap_db_storage_locations(File user_data_dir, out File db_file,
- out File attachments_dir) {
- db_file = ImapDB.Database.get_db_file(user_data_dir);
- attachments_dir = ImapDB.Attachment.get_attachments_dir(user_data_dir);
- }
-
+
public async void open_async(File user_data_dir, File schema_dir, Cancellable? cancellable)
throws Error {
- if (db != null)
+ if (this.db != null)
throw new EngineError.ALREADY_OPEN("IMAP database already open");
-
- db = new ImapDB.Database(user_data_dir, schema_dir, upgrade_monitor, vacuum_monitor,
- account_information.primary_mailbox.address);
-
+
+ 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,
+ account_information.primary_mailbox.address
+ );
+
try {
yield db.open(
Db.DatabaseFlags.CREATE_DIRECTORY | Db.DatabaseFlags.CREATE_FILE |
Db.DatabaseFlags.CHECK_CORRUPTION,
@@ -650,28 +670,30 @@ private class Geary.ImapDB.Account : BaseObject {
// return current if already created
ImapDB.Folder? folder = get_local_folder(path);
if (folder != null) {
- // update properties
folder.set_properties(properties);
-
- return folder;
+ } else {
+ folder = new Geary.ImapDB.Folder(
+ db,
+ path,
+ db.attachments_path,
+ contact_store,
+ account_information.primary_mailbox.address,
+ folder_id,
+ properties
+ );
+
+ // build a reference to it
+ FolderReference folder_ref = new FolderReference(folder, path);
+ folder_ref.reference_broken.connect(on_folder_reference_broken);
+
+ // add to the references table
+ folder_refs.set(folder_ref.path, folder_ref);
+
+ folder.unread_updated.connect(on_unread_updated);
}
-
- // create folder
- folder = new Geary.ImapDB.Folder(db, path, contact_store,
account_information.primary_mailbox.address, folder_id,
- properties);
-
- // build a reference to it
- FolderReference folder_ref = new FolderReference(folder, path);
- folder_ref.reference_broken.connect(on_folder_reference_broken);
-
- // add to the references table
- folder_refs.set(folder_ref.path, folder_ref);
-
- folder.unread_updated.connect(on_unread_updated);
-
return folder;
}
-
+
private void on_folder_reference_broken(Geary.SmartReference reference) {
FolderReference folder_ref = (FolderReference) reference;
@@ -706,8 +728,10 @@ private class Geary.ImapDB.Account : BaseObject {
// Ignore any messages that don't have the required fields.
if (partial_ok || row.fields.fulfills(requested_fields)) {
Geary.Email email = row.to_email(new Geary.ImapDB.EmailIdentifier(id, null));
- Geary.ImapDB.Folder.do_add_attachments(cx, email, id, cancellable);
-
+ Geary.ImapDB.Folder.do_add_attachments(
+ cx, this.db.attachments_path, email, id, cancellable
+ );
+
Gee.Set<Geary.FolderPath>? folders = do_find_email_folders(cx, id, true, cancellable);
if (folders == null) {
if (folder_blacklist == null || !folder_blacklist.contains(null))
@@ -1367,10 +1391,12 @@ private class Geary.ImapDB.Account : BaseObject {
throw new EngineError.INCOMPLETE_MESSAGE(
"Message %s only fulfills %Xh fields (required: %Xh)",
email_id.to_string(), row.fields, required_fields);
-
+
email = row.to_email(email_id);
- Geary.ImapDB.Folder.do_add_attachments(cx, email, email_id.message_id, cancellable);
-
+ Geary.ImapDB.Folder.do_add_attachments(
+ cx, this.db.attachments_path, email, email_id.message_id, cancellable
+ );
+
return Db.TransactionOutcome.DONE;
}, cancellable);
@@ -1530,8 +1556,10 @@ private class Geary.ImapDB.Account : BaseObject {
MessageRow row = Geary.ImapDB.Folder.do_fetch_message_row(
cx, message_id, search_fields, out db_fields, cancellable);
Geary.Email email = row.to_email(new Geary.ImapDB.EmailIdentifier(message_id, null));
- Geary.ImapDB.Folder.do_add_attachments(cx, email, message_id, cancellable);
-
+ Geary.ImapDB.Folder.do_add_attachments(
+ cx, this.db.attachments_path, email, message_id, cancellable
+ );
+
Geary.ImapDB.Folder.do_add_email_to_search_table(cx, message_id, email, cancellable);
} catch (Error e) {
// This is a somewhat serious issue since we rely on
diff --git a/src/engine/imap-db/imap-db-attachment.vala b/src/engine/imap-db/imap-db-attachment.vala
index 74fde3f..c7752d1 100644
--- a/src/engine/imap-db/imap-db-attachment.vala
+++ b/src/engine/imap-db/imap-db-attachment.vala
@@ -8,7 +8,6 @@ private class Geary.ImapDB.Attachment : Geary.Attachment {
public const Email.Field REQUIRED_FIELDS = Email.REQUIRED_FOR_MESSAGE;
internal const string NULL_FILE_NAME = "none";
- private const string ATTACHMENTS_DIR = "attachments";
public Attachment(int64 message_id,
int64 attachment_id,
@@ -33,15 +32,12 @@ private class Geary.ImapDB.Attachment : Geary.Attachment {
return "imap-db:%s".printf(attachment_id.to_string());
}
- public static File generate_file(File data_dir, int64 message_id, int64 attachment_id,
+ public static File generate_file(File attachements_dir, int64 message_id, int64 attachment_id,
string? filename) {
- return get_attachments_dir(data_dir)
+ return attachements_dir
.get_child(message_id.to_string())
.get_child(attachment_id.to_string())
.get_child(filename ?? NULL_FILE_NAME);
}
- public static File get_attachments_dir(File data_dir) {
- return data_dir.get_child(ATTACHMENTS_DIR);
- }
}
diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala
index d7686d2..bced663 100644
--- a/src/engine/imap-db/imap-db-database.vala
+++ b/src/engine/imap-db/imap-db-database.vala
@@ -8,32 +8,32 @@
extern int sqlite3_unicodesn_register_tokenizer(Sqlite.Database db);
private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
- private const string DB_FILENAME = "geary.db";
+
+ internal GLib.File attachments_path;
+
private const int OPEN_PUMP_EVENT_LOOP_MSEC = 100;
-
+
private ProgressMonitor upgrade_monitor;
private ProgressMonitor vacuum_monitor;
private string account_owner_email;
private bool new_db = false;
private Cancellable gc_cancellable = new Cancellable();
- public Database(GLib.File db_dir,
+ public Database(GLib.File db_file,
GLib.File schema_dir,
+ GLib.File attachments_path,
ProgressMonitor upgrade_monitor,
ProgressMonitor vacuum_monitor,
string account_owner_email) {
- base.persistent(get_db_file(db_dir), schema_dir);
+ base.persistent(db_file, schema_dir);
+ this.attachments_path = attachments_path;
this.upgrade_monitor = upgrade_monitor;
this.vacuum_monitor = vacuum_monitor;
// Update to use all addresses on the account. Bug 768779
this.account_owner_email = account_owner_email;
}
-
- public static File get_db_file(File db_dir) {
- return db_dir.get_child(DB_FILENAME);
- }
-
+
/**
* Prepares the ImapDB database for use.
*/
@@ -336,8 +336,13 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
Mime.DispositionType target_disposition = Mime.DispositionType.UNSPECIFIED;
if (message.get_sub_messages().is_empty)
target_disposition = Mime.DispositionType.INLINE;
- Geary.ImapDB.Folder.do_save_attachments_db(cx, id,
- message.get_attachments(target_disposition), this, null);
+ Geary.ImapDB.Folder.do_save_attachments_db(
+ cx,
+ id,
+ message.get_attachments(target_disposition),
+ this.attachments_path,
+ null
+ );
} catch (Error e) {
debug("Error fetching inline Mime parts: %s", e.message);
}
@@ -495,7 +500,9 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
message.get_attachments();
try {
- Geary.ImapDB.Folder.do_delete_attachments(cx, message_id);
+ Geary.ImapDB.Folder.do_delete_attachments(
+ cx, this.attachments_path, message_id
+ );
} catch (Error err) {
debug("Error deleting existing attachments: %s",
err.message);
@@ -504,8 +511,13 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
// rebuild all
try {
- Geary.ImapDB.Folder.do_save_attachments_db(cx, message_id, msg_attachments,
- this, null);
+ Geary.ImapDB.Folder.do_save_attachments_db(
+ cx,
+ message_id,
+ msg_attachments,
+ this.attachments_path,
+ null
+ );
} catch (Error err) {
debug("Error saving attachments: %s", err.message);
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index 0d4dcd8..f7aa264 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -82,9 +82,10 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
protected int manual_ref_count { get; protected set; }
-
- private ImapDB.Database db;
+
+ private Geary.Db.Database db;
private Geary.FolderPath path;
+ private GLib.File attachments_path;
private ContactStore contact_store;
private string account_owner_email;
private int64 folder_id;
@@ -102,14 +103,16 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
*/
public signal void unread_updated(Gee.Map<ImapDB.EmailIdentifier, bool> unread_status);
- internal Folder(ImapDB.Database db,
+ internal Folder(Geary.Db.Database db,
Geary.FolderPath path,
+ GLib.File attachments_path,
ContactStore contact_store,
string account_owner_email,
int64 folder_id,
Geary.Imap.FolderProperties properties) {
this.db = db;
this.path = path;
+ this.attachments_path = attachments_path;
this.contact_store = contact_store;
// Update to use all addresses on the account. Bug 768779
this.account_owner_email = account_owner_email;
@@ -1593,16 +1596,23 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
Geary.Email email = row.to_email(location.email_id);
-
- return do_add_attachments(cx, email, location.message_id, cancellable);
+
+ return do_add_attachments(
+ cx, this.attachments_path, email, location.message_id, cancellable
+ );
}
-
- internal static Geary.Email do_add_attachments(Db.Connection cx, Geary.Email email,
- int64 message_id, Cancellable? cancellable = null) throws Error {
+
+ internal static Geary.Email do_add_attachments(Db.Connection cx,
+ GLib.File attachments_path,
+ Geary.Email email,
+ int64 message_id,
+ Cancellable? cancellable = null)
+ throws Error {
// Add attachments if available
if (email.fields.fulfills(ImapDB.Attachment.REQUIRED_FIELDS)) {
- Gee.List<Geary.Attachment>? attachments = do_list_attachments(cx, message_id,
- cancellable);
+ Gee.List<Geary.Attachment>? attachments = do_list_attachments(
+ cx, attachments_path, message_id, cancellable
+ );
if (attachments != null)
email.add_attachments(attachments);
}
@@ -2038,11 +2048,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
do_save_attachments(cx, location.message_id, combined_email.get_message().get_attachments(),
cancellable);
}
-
+
// Must add attachments to the email object after they're saved to
// the database.
- do_add_attachments(cx, combined_email, location.message_id, cancellable);
-
+ do_add_attachments(
+ cx,
+ this.attachments_path,
+ combined_email,
+ location.message_id,
+ cancellable
+ );
+
Geary.Email.Field new_fields;
do_merge_message_row(cx, row, out new_fields, out updated_contacts,
ref new_unread_count, cancellable);
@@ -2061,9 +2077,13 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
unread_count_change += new_unread_count;
}
-
- private static Gee.List<Geary.Attachment>? do_list_attachments(Db.Connection cx, int64 message_id,
- Cancellable? cancellable) throws Error {
+
+ private static Gee.List<Geary.Attachment>?
+ do_list_attachments(Db.Connection cx,
+ GLib.File attachments_path,
+ int64 message_id,
+ Cancellable? cancellable)
+ throws Error {
Db.Statement stmt = cx.prepare("""
SELECT id, filename, mime_type, filesize, disposition, content_id, description
FROM MessageAttachmentTable
@@ -2097,7 +2117,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
results.string_at(6),
disposition,
content_filename,
- cx.database.file.get_parent(),
+ attachments_path,
results.int64_at(3)
)
);
@@ -2108,11 +2128,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
private void do_save_attachments(Db.Connection cx, int64 message_id,
Gee.List<GMime.Part>? attachments, Cancellable? cancellable) throws Error {
- do_save_attachments_db(cx, message_id, attachments, db, cancellable);
+ do_save_attachments_db(
+ cx, message_id, attachments, this.attachments_path, cancellable
+ );
}
-
- public static void do_save_attachments_db(Db.Connection cx, int64 message_id,
- Gee.List<GMime.Part>? attachments, ImapDB.Database db, Cancellable? cancellable) throws Error {
+
+ public static void do_save_attachments_db(Db.Connection cx,
+ int64 message_id,
+ Gee.List<GMime.Part>? attachments,
+ GLib.File attachments_path,
+ Cancellable? cancellable)
+ throws Error {
// nothing to do if no attachments
if (attachments == null || attachments.size == 0)
return;
@@ -2156,11 +2182,11 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_int(4, disposition_type);
stmt.bind_string(5, content_id);
stmt.bind_string(6, description);
-
- int64 attachment_id = stmt.exec_insert(cancellable);
- File saved_file = ImapDB.Attachment.generate_file(db.file.get_parent(), message_id,
- attachment_id, filename);
+ int64 attachment_id = stmt.exec_insert(cancellable);
+ File saved_file = ImapDB.Attachment.generate_file(
+ attachments_path, message_id, attachment_id, filename
+ );
// On the off-chance this is marked for deletion, unmark it
try {
@@ -2231,13 +2257,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
}
}
-
- public static void do_delete_attachments(Db.Connection cx, int64 message_id)
+
+ public static void do_delete_attachments(Db.Connection cx,
+ GLib.File attachments_path,
+ int64 message_id)
throws Error {
- Gee.List<Geary.Attachment>? attachments = do_list_attachments(cx, message_id, null);
+ Gee.List<Geary.Attachment>? attachments = do_list_attachments(
+ cx, attachments_path, message_id, null
+ );
if (attachments == null || attachments.size == 0)
return;
-
+
// delete all files
foreach (Geary.Attachment attachment in attachments) {
try {
diff --git a/src/engine/imap-db/imap-db-gc.vala b/src/engine/imap-db/imap-db-gc.vala
index b567cb4..36c7870 100644
--- a/src/engine/imap-db/imap-db-gc.vala
+++ b/src/engine/imap-db/imap-db-gc.vala
@@ -83,12 +83,10 @@ private class Geary.ImapDB.GC {
private ImapDB.Database db;
private int priority;
- private File data_dir;
-
+
public GC(ImapDB.Database db, int priority) {
this.db = db;
this.priority = priority;
- data_dir = db.file.get_parent();
}
/**
@@ -432,10 +430,14 @@ private class Geary.ImapDB.GC {
result = stmt.exec(cancellable);
while (!result.finished) {
- File file = Attachment.generate_file(data_dir, message_id, result.rowid_for("id"),
- result.string_for("filename"));
+ File file = Attachment.generate_file(
+ this.db.attachments_path,
+ message_id,
+ result.rowid_for("id"),
+ result.string_for("filename")
+ );
attachment_files.add(file);
-
+
result.next(cancellable);
}
@@ -572,7 +574,7 @@ 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.file.get_parent());
+ File current_dir = current ?? db.attachments_path;
// directory is considered empty until file or non-deleted child directory is found
empty = true;
diff --git a/test/engine/imap-db/imap-db-database-test.vala b/test/engine/imap-db/imap-db-database-test.vala
index 209694a..21c61ad 100644
--- a/test/engine/imap-db/imap-db-database-test.vala
+++ b/test/engine/imap-db/imap-db-database-test.vala
@@ -20,8 +20,9 @@ class Geary.ImapDB.DatabaseTest : TestCase {
);
Database db = new Database(
- tmp_dir,
+ tmp_dir.get_child("test.db"),
GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql"),
+ tmp_dir.get_child("attachments"),
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_UPGRADE),
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_VACUUM),
"test example com"
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
