Remove existing contact harvesting mechanism

Remove contact harvesting from DB version 005 (version 0.1.1), allowing
the implementation of both ContactStoreImpl and ImapDB.Database to be
cleaned up. Remove contact collection from ImapDB.Folder, reducing the
numer of Gee objects created when creating/merging messages. Finally,
remove the MessageAddresses object now that it is unused.
This commit is contained in:
Michael Gratton 2019-06-07 18:06:31 +10:00 committed by Michael James Gratton
parent 809f664319
commit cb8150ce03
10 changed files with 101 additions and 306 deletions

View file

@ -222,7 +222,6 @@ src/engine/imap-db/imap-db-database.vala
src/engine/imap-db/imap-db-email-identifier.vala
src/engine/imap-db/imap-db-folder.vala
src/engine/imap-db/imap-db-gc.vala
src/engine/imap-db/imap-db-message-addresses.vala
src/engine/imap-db/imap-db-message-row.vala
src/engine/imap-db/search/imap-db-search-email-identifier.vala
src/engine/imap-db/search/imap-db-search-folder-properties.vala

View file

@ -1,6 +1,6 @@
--
-- Create ContactTable for autocompletion contacts. Data is migrated in vala upgrade hooks.
-- Create ContactTable for autocompletion contacts.
--
CREATE TABLE ContactTable (

View file

@ -12,15 +12,46 @@
internal class Geary.ContactStoreImpl : BaseObject, Geary.ContactStore {
// Insert or update a contact in the ContactTable. If contact
// already exists, flags are merged and the importance is updated
// to the highest importance seen.
//
// Internal and static since it is used by ImapDB.Database during
// upgrades
internal static void do_update_contact(Db.Connection cx,
Contact updated,
GLib.Cancellable? cancellable)
private Geary.Db.Database backing;
internal ContactStoreImpl(Geary.Db.Database backing) {
base_ref();
this.backing = backing;
}
/** Returns the contact matching the given email address, if any */
public async Contact? get_by_rfc822(Geary.RFC822.MailboxAddress mailbox,
GLib.Cancellable? cancellable)
throws GLib.Error {
Contact? contact = null;
yield this.backing.exec_transaction_async(
Db.TransactionType.RO,
(cx, cancellable) => {
contact = do_fetch_contact(cx, mailbox.address, cancellable);
return Db.TransactionOutcome.COMMIT;
},
cancellable);
return contact;
}
public async void update_contacts(Gee.Collection<Contact> updated,
GLib.Cancellable? cancellable)
throws GLib.Error {
yield this.backing.exec_transaction_async(
Db.TransactionType.RW,
(cx, cancellable) => {
foreach (Contact contact in updated) {
do_update_contact(cx, contact, cancellable);
}
return Db.TransactionOutcome.COMMIT;
},
cancellable);
}
private void do_update_contact(Db.Connection cx,
Contact updated,
GLib.Cancellable? cancellable)
throws GLib.Error {
Contact? existing = do_fetch_contact(
cx, updated.email, cancellable
@ -69,11 +100,9 @@ internal class Geary.ContactStoreImpl : BaseObject, Geary.ContactStore {
}
}
// Static since it is indirectly used by ImapDB.Database during
// upgrades
private static Contact? do_fetch_contact(Db.Connection cx,
string email,
GLib.Cancellable? cancellable)
private Contact? do_fetch_contact(Db.Connection cx,
string email,
GLib.Cancellable? cancellable)
throws GLib.Error {
Db.Statement stmt = cx.prepare(
"SELECT real_name, highest_importance, normalized_email, flags FROM ContactTable "
@ -95,42 +124,4 @@ internal class Geary.ContactStoreImpl : BaseObject, Geary.ContactStore {
return contact;
}
private Geary.Db.Database backing;
internal ContactStoreImpl(Geary.Db.Database backing) {
base_ref();
this.backing = backing;
}
/** Returns the contact matching the given email address, if any */
public async Contact? get_by_rfc822(Geary.RFC822.MailboxAddress mailbox,
GLib.Cancellable? cancellable)
throws GLib.Error {
Contact? contact = null;
yield this.backing.exec_transaction_async(
Db.TransactionType.RO,
(cx, cancellable) => {
contact = do_fetch_contact(cx, mailbox.address, cancellable);
return Db.TransactionOutcome.COMMIT;
},
cancellable);
return contact;
}
public async void update_contacts(Gee.Collection<Contact> updated,
GLib.Cancellable? cancellable)
throws GLib.Error {
yield this.backing.exec_transaction_async(
Db.TransactionType.RW,
(cx, cancellable) => {
foreach (Contact contact in updated) {
do_update_contact(cx, contact, cancellable);
}
return Db.TransactionOutcome.COMMIT;
},
cancellable);
}
}

View file

@ -89,7 +89,6 @@ private class Geary.ImapDB.Account : BaseObject {
}
// Only available when the Account is opened
public ContactStore contact_store { get; private set; }
public IntervalProgressMonitor search_index_monitor { get; private set;
default = new IntervalProgressMonitor(ProgressType.SEARCH_INDEX, 0, 0); }
public SimpleProgressMonitor upgrade_monitor { get; private set; default = new SimpleProgressMonitor(
@ -288,8 +287,7 @@ private class Geary.ImapDB.Account : BaseObject {
schema_dir,
attachments_dir,
upgrade_monitor,
vacuum_monitor,
account_information.primary_mailbox.address
vacuum_monitor
);
try {
@ -351,8 +349,6 @@ private class Geary.ImapDB.Account : BaseObject {
return false;
});
this.contact_store = new ContactStoreImpl(db);
}
public async void close_async(Cancellable? cancellable) throws Error {
@ -617,7 +613,6 @@ private class Geary.ImapDB.Account : BaseObject {
db,
path,
db.attachments_path,
contact_store,
account_information.primary_mailbox.address,
folder_id,
properties

View file

@ -15,7 +15,6 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
private ProgressMonitor upgrade_monitor;
private ProgressMonitor vacuum_monitor;
private string account_owner_email;
private bool new_db = false;
private GC? gc = null;
@ -25,15 +24,11 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
GLib.File schema_dir,
GLib.File attachments_path,
ProgressMonitor upgrade_monitor,
ProgressMonitor vacuum_monitor,
string account_owner_email) {
ProgressMonitor vacuum_monitor) {
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;
}
/**
@ -133,10 +128,6 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
Cancellable? cancellable)
throws Error {
switch (version) {
case 5:
yield post_upgrade_populate_autocomplete(cancellable);
break;
case 6:
yield post_upgrade_encode_folder_names(cancellable);
break;
@ -179,25 +170,6 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
}
}
// Version 5.
private async void post_upgrade_populate_autocomplete(Cancellable? cancellable)
throws Error {
yield exec_transaction_async(Db.TransactionType.RW, (cx) => {
Db.Result result = cx.query(
"SELECT sender, from_field, to_field, cc, bcc FROM MessageTable"
);
while (!result.finished && !cancellable.is_cancelled()) {
MessageAddresses message_addresses =
new MessageAddresses.from_result(account_owner_email, result);
foreach (Contact contact in message_addresses.contacts) {
ContactStoreImpl.do_update_contact(cx, contact, null);
}
result.next();
}
return Geary.Db.TransactionOutcome.COMMIT;
}, cancellable);
}
// Version 6.
private async void post_upgrade_encode_folder_names(Cancellable? cancellable)
throws Error {

View file

@ -91,7 +91,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
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;
private Geary.Imap.FolderProperties properties;
@ -111,14 +110,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
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;
this.folder_id = folder_id;
@ -290,19 +287,18 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Gee.List<Geary.Email> slice = list.slice(index, stop);
Gee.ArrayList<Geary.EmailIdentifier> complete_ids = new Gee.ArrayList<Geary.EmailIdentifier>();
Gee.Collection<Contact> updated_contacts = new Gee.ArrayList<Contact>();
int total_unread_change = 0;
yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => {
foreach (Geary.Email email in slice) {
Gee.Collection<Contact>? contacts_this_email = null;
Geary.Email.Field pre_fields;
Geary.Email.Field post_fields;
int unread_change = 0;
bool created = do_create_or_merge_email(cx, email, out pre_fields,
out post_fields, out contacts_this_email, ref unread_change, cancellable);
if (contacts_this_email != null)
updated_contacts.add_all(contacts_this_email);
bool created = do_create_or_merge_email(
cx, email,
out pre_fields, out post_fields,
ref unread_change,
cancellable
);
results.set(email, created);
@ -321,12 +317,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return Db.TransactionOutcome.COMMIT;
}, cancellable);
if (updated_contacts.size > 0) {
yield this.contact_store.update_contacts(
updated_contacts, cancellable
);
}
if (update_totals) {
// Update the email_unread properties.
properties.set_status_unseen(
@ -1373,10 +1363,13 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
*
* Returns `true` if created, else was merged and returns `false`.
*/
private bool do_create_or_merge_email(Db.Connection cx, Geary.Email email,
out Geary.Email.Field pre_fields, out Geary.Email.Field post_fields,
out Gee.Collection<Contact> updated_contacts, ref int unread_count_change,
Cancellable? cancellable) throws Error {
private bool do_create_or_merge_email(Db.Connection cx,
Geary.Email email,
out Geary.Email.Field pre_fields,
out Geary.Email.Field post_fields,
ref int unread_count_change,
GLib.Cancellable? cancellable)
throws GLib.Error {
// This should only ever get invoked for messages that came
// from the IMAP layer, which should not have a message id,
@ -1424,15 +1417,22 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// special-case flag-only updates, which happens often and
// will only write to the DB if necessary.
if (email.fields != Geary.Email.Field.FLAGS) {
do_merge_email(cx, location, email, out pre_fields, out post_fields,
out updated_contacts, ref unread_count_change, cancellable);
do_merge_email(
cx, location, email,
out pre_fields, out post_fields,
ref unread_count_change,
cancellable
);
// Already associated with folder and flags were known.
if (is_associated && pre_fields.is_all_set(Geary.Email.Field.FLAGS))
unread_count_change = 0;
} else {
do_merge_email_flags(cx, location, email, out pre_fields, out post_fields,
out updated_contacts, ref unread_count_change, cancellable);
do_merge_email_flags(
cx, location, email,
out pre_fields, out post_fields,
ref unread_count_change, cancellable
);
}
} else {
// Message was not found, so create a new message for it
@ -1485,12 +1485,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
do_add_email_to_search_table(cx, message_id, email, cancellable);
MessageAddresses message_addresses =
new MessageAddresses.from_email(account_owner_email, email);
foreach (Contact contact in message_addresses.contacts)
do_update_contact(cx, contact, cancellable);
updated_contacts = message_addresses.contacts;
// Update unread count if our new email is unread.
if (email.email_flags != null && email.email_flags.is_unread())
unread_count_change++;
@ -1782,13 +1776,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return true;
}
private void do_merge_message_row(Db.Connection cx, MessageRow row,
out Geary.Email.Field new_fields, out Gee.Collection<Contact> updated_contacts,
ref int unread_count_change, Cancellable? cancellable) throws Error {
// Initialize to an empty list, in case we return early.
updated_contacts = new Gee.LinkedList<Contact>();
private void do_merge_message_row(Db.Connection cx,
MessageRow row,
out Geary.Email.Field new_fields,
ref int unread_count_change,
GLib.Cancellable? cancellable)
throws GLib.Error {
Geary.Email.Field available_fields;
if (!do_fetch_email_fields(cx, row.id, out available_fields, cancellable))
throw new EngineError.NOT_FOUND("No message with ID %s found in database", row.id.to_string());
@ -1918,13 +1911,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_rowid(1, row.id);
stmt.exec(cancellable);
// Update the autocompletion table.
MessageAddresses message_addresses =
new MessageAddresses.from_row(account_owner_email, row);
foreach (Geary.Contact contact in message_addresses.contacts)
do_update_contact(cx, contact, cancellable);
updated_contacts = message_addresses.contacts;
}
private void do_merge_email_in_search_table(Db.Connection cx, int64 message_id,
@ -2007,15 +1993,16 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// This *replaces* the stored flags, it does not OR them ... this is simply a fast-path over
// do_merge_email(), as updating FLAGS happens often and doesn't require a lot of extra work
private void do_merge_email_flags(Db.Connection cx, LocationIdentifier location, Geary.Email email,
out Geary.Email.Field pre_fields, out Geary.Email.Field post_fields,
out Gee.Collection<Contact> updated_contacts, ref int unread_count_change,
Cancellable? cancellable) throws Error {
private void do_merge_email_flags(Db.Connection cx,
LocationIdentifier location,
Geary.Email email,
out Geary.Email.Field pre_fields,
out Geary.Email.Field post_fields,
ref int unread_count_change,
GLib.Cancellable? cancellable)
throws GLib.Error {
assert(email.fields == Geary.Email.Field.FLAGS);
// no contacts were harmed in the production of this email
updated_contacts = new Gee.ArrayList<Contact>();
// fetch MessageRow and its fields, note that the fields now include FLAGS if they didn't
// already
MessageRow row = do_fetch_message_row(cx, location.message_id, Geary.Email.Field.FLAGS,
@ -2050,13 +2037,14 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
}
private void do_merge_email(Db.Connection cx, LocationIdentifier location, Geary.Email email,
out Geary.Email.Field pre_fields, out Geary.Email.Field post_fields,
out Gee.Collection<Contact> updated_contacts, ref int unread_count_change,
Cancellable? cancellable) throws Error {
// Default to an empty list, in case we never call do_merge_message_row.
updated_contacts = new Gee.LinkedList<Contact>();
private void do_merge_email(Db.Connection cx,
LocationIdentifier location,
Geary.Email email,
out Geary.Email.Field pre_fields,
out Geary.Email.Field post_fields,
ref int unread_count_change,
GLib.Cancellable? cancellable)
throws GLib.Error {
// fetch message from database and merge in this email
MessageRow row = do_fetch_message_row(cx, location.message_id,
email.fields | Email.REQUIRED_FOR_MESSAGE | Attachment.REQUIRED_FIELDS,
@ -2090,8 +2078,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
Geary.Email.Field new_fields;
do_merge_message_row(cx, row, out new_fields, out updated_contacts,
ref new_unread_count, cancellable);
do_merge_message_row(
cx, row,
out new_fields,
ref new_unread_count,
cancellable
);
if (do_check_for_message_search_row(cx, location.message_id, cancellable))
do_merge_email_in_search_table(cx, location.message_id, new_fields, combined_email, cancellable);

View file

@ -1,151 +0,0 @@
/* Copyright 2016 Software Freedom Conservancy Inc.
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
private class Geary.ImapDB.MessageAddresses : BaseObject {
// Read-only view.
public Gee.Collection<Contact> contacts { get; private set; }
private RFC822.MailboxAddress? sender_address;
private RFC822.MailboxAddresses? from_addresses;
private RFC822.MailboxAddresses? to_addresses;
private RFC822.MailboxAddresses? cc_addresses;
private RFC822.MailboxAddresses? bcc_addresses;
private int from_importance;
private int to_importance;
private int cc_importance;
private MessageAddresses(string account_owner_email, RFC822.MailboxAddress? sender_address,
RFC822.MailboxAddresses? from_addresses, RFC822.MailboxAddresses? to_addresses,
RFC822.MailboxAddresses? cc_addresses, RFC822.MailboxAddresses? bcc_addresses) {
this.sender_address = sender_address;
this.from_addresses = from_addresses;
this.to_addresses = to_addresses;
this.cc_addresses = cc_addresses;
this.bcc_addresses = bcc_addresses;
calculate_importance(account_owner_email);
contacts = build_contacts();
}
private MessageAddresses.from_strings(string account_owner_email, string? sender_field,
string? from_field, string? to_field, string? cc_field, string? bcc_field) {
this(account_owner_email, get_address_from_string(sender_field),
get_addresses_from_string(from_field), get_addresses_from_string(to_field),
get_addresses_from_string(cc_field), get_addresses_from_string(bcc_field));
}
public MessageAddresses.from_email(string account_owner_email, Geary.Email email) {
this(account_owner_email, email.sender, email.from, email.to, email.cc, email.bcc);
}
public MessageAddresses.from_row(string account_owner_email, MessageRow row) {
this.from_strings(account_owner_email, row.sender, row.from, row.to, row.cc, row.bcc);
}
public MessageAddresses.from_result(string account_owner_email, Db.Result result) {
this.from_strings(account_owner_email, get_string_or_null(result, "sender"),
get_string_or_null(result, "from_field"), get_string_or_null(result, "to_field"),
get_string_or_null(result, "cc"), get_string_or_null(result, "bcc"));
}
private static string? get_string_or_null(Db.Result result, string column) {
try {
return result.string_for(column);
} catch (Geary.DatabaseError err) {
debug("Error fetching addresses from message row: %s", err.message);
return null;
}
}
private static RFC822.MailboxAddress? get_address_from_string(string? field) {
RFC822.MailboxAddress? addr = null;
if (field != null) {
try {
new RFC822.MailboxAddress.from_rfc822_string(field);
} catch (RFC822Error e) {
// oh well
}
}
return addr;
}
private static RFC822.MailboxAddresses? get_addresses_from_string(string? field) {
return field == null ? null : new RFC822.MailboxAddresses.from_rfc822_string(field);
}
private void calculate_importance(string account_owner_email) {
// "Sender" is different than "from", but we give it the same importance.
bool account_owner_in_from =
(sender_address != null && sender_address.equal_normalized(account_owner_email)) ||
(from_addresses != null && from_addresses.contains_normalized(account_owner_email));
bool account_owner_in_to = to_addresses != null &&
to_addresses.contains_normalized(account_owner_email);
// If the account owner's address does not appear in any of these fields, we assume they
// were BCC'd.
bool account_owner_in_cc =
(cc_addresses != null && cc_addresses.contains_normalized(account_owner_email)) ||
(bcc_addresses != null && bcc_addresses.contains_normalized(account_owner_email)) ||
!(account_owner_in_from || account_owner_in_to);
from_importance = -1;
to_importance = -1;
cc_importance = -1;
if (account_owner_in_from) {
from_importance = int.max(from_importance, ContactImportance.FROM_FROM);
to_importance = int.max(to_importance, ContactImportance.FROM_TO);
cc_importance = int.max(cc_importance, ContactImportance.FROM_CC);
}
if (account_owner_in_to) {
from_importance = int.max(from_importance, ContactImportance.TO_FROM);
to_importance = int.max(to_importance, ContactImportance.TO_TO);
cc_importance = int.max(cc_importance, ContactImportance.TO_CC);
}
if (account_owner_in_cc) {
from_importance = int.max(from_importance, ContactImportance.CC_FROM);
to_importance = int.max(to_importance, ContactImportance.CC_TO);
cc_importance = int.max(cc_importance, ContactImportance.CC_CC);
}
}
private Gee.Collection<Contact> build_contacts() {
Gee.Map<string, Contact> contacts_map = new Gee.HashMap<string, Contact>();
if (this.sender_address != null) {
add_contact(contacts_map, this.sender_address, this.from_importance);
}
add_contacts(contacts_map, this.from_addresses, this.from_importance);
add_contacts(contacts_map, this.to_addresses, this.to_importance);
add_contacts(contacts_map, this.cc_addresses, this.cc_importance);
add_contacts(contacts_map, this.bcc_addresses, this.cc_importance);
return contacts_map.values;
}
private void add_contacts(Gee.Map<string, Contact> contacts_map, RFC822.MailboxAddresses? addresses,
int importance) {
if (addresses == null)
return;
foreach (RFC822.MailboxAddress address in addresses)
add_contact(contacts_map, address, importance);
}
private void add_contact(Gee.Map<string, Contact> contacts_map, RFC822.MailboxAddress address,
int importance) {
if (!address.is_valid())
return;
Contact contact = new Contact.from_rfc822_address(address, importance);
Contact? old_contact = contacts_map[contact.normalized_email];
if (old_contact == null || old_contact.highest_importance < contact.highest_importance)
contacts_map[contact.normalized_email] = contact;
}
}

View file

@ -83,7 +83,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
base(config, imap, smtp);
this.local = local;
this.contact_store = local.contact_store;
this.contact_store = new ContactStoreImpl(local.db);
imap.min_pool_size = IMAP_MIN_POOL_SIZE;
imap.notify["current-status"].connect(

View file

@ -176,7 +176,6 @@ geary_engine_vala_sources = files(
'imap-db/imap-db-email-identifier.vala',
'imap-db/imap-db-folder.vala',
'imap-db/imap-db-gc.vala',
'imap-db/imap-db-message-addresses.vala',
'imap-db/imap-db-message-row.vala',
'imap-db/search/imap-db-search-email-identifier.vala',
'imap-db/search/imap-db-search-folder.vala',

View file

@ -35,8 +35,7 @@ class Geary.ImapDB.DatabaseTest : TestCase {
GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql"),
this.tmp_dir.get_child("attachments"),
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_UPGRADE),
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_VACUUM),
"test@example.com"
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_VACUUM)
);
db.open.begin(
@ -96,8 +95,7 @@ class Geary.ImapDB.DatabaseTest : TestCase {
GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql"),
attachments_dir,
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_UPGRADE),
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_VACUUM),
"test@example.com"
new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_VACUUM)
);
db.open.begin(