Commit 292498b0 authored by Michael Gratton's avatar Michael Gratton 🤞

Merge branch 'wip/email-flag-refinement-redux' into 'master'

Email flag refinement redux

Closes #213

See merge request !107
parents 72ba5e68 744cde0c
Pipeline #59345 failed with stages
in 7 minutes and 13 seconds
......@@ -188,8 +188,12 @@ public abstract class Geary.Folder : BaseObject {
/**
* Direction of list traversal (if not set, from newest to oldest).
*/
OLDEST_TO_NEWEST;
OLDEST_TO_NEWEST,
/**
* Internal use only, prevents flag changes updating unread count.
*/
NO_UNREAD_UPDATE;
public bool is_any_set(ListFlags flags) {
return (this & flags) != 0;
}
......
......@@ -20,7 +20,16 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
/**
* Fields required for a message to be stored in the database.
*/
public const Geary.Email.Field REQUIRED_FIELDS = Geary.Email.Field.PROPERTIES|Email.Field.REFERENCES;
public const Geary.Email.Field REQUIRED_FIELDS = (
// Required for primary duplicate detection done with properties
Email.Field.PROPERTIES |
// Required for secondary duplicate detection via UID
Email.Field.REFERENCES |
// Required to ensure the unread count is up to date and so
// that when moving a message, the new copy turns back up as
// being not deleted.
Email.Field.FLAGS
);
/**
* Fields required for a message to be considered for full-text indexing.
......@@ -276,8 +285,11 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// function (see ImapDB.EmailIdentifier.promote_with_message_id). This
// means if you've hashed the collection of EmailIdentifiers prior, you may
// not be able to find them after this function. Be warned.
public async Gee.Map<Geary.Email, bool> create_or_merge_email_async(Gee.Collection<Geary.Email> emails,
Cancellable? cancellable) throws Error {
public async Gee.Map<Email, bool>
create_or_merge_email_async(Gee.Collection<Email> emails,
bool update_totals,
GLib.Cancellable? cancellable)
throws GLib.Error {
Gee.HashMap<Geary.Email, bool> results = new Gee.HashMap<Geary.Email, bool>();
Gee.ArrayList<Geary.Email> list = traverse<Geary.Email>(emails).to_array_list();
......@@ -307,22 +319,27 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// have all the fields but after the create/merge now does
if (post_fields.is_all_set(Geary.Email.Field.ALL) && !pre_fields.is_all_set(Geary.Email.Field.ALL))
complete_ids.add(email.id);
// Update unread count in DB.
do_add_to_unread_count(cx, unread_change, cancellable);
total_unread_change += unread_change;
if (update_totals) {
// Update unread count in DB.
do_add_to_unread_count(cx, unread_change, cancellable);
total_unread_change += unread_change;
}
}
return Db.TransactionOutcome.COMMIT;
}, cancellable);
if (updated_contacts.size > 0)
contact_store.update_contacts(updated_contacts);
// Update the email_unread properties.
properties.set_status_unseen((properties.email_unread + total_unread_change).clamp(0, int.MAX));
if (update_totals) {
// Update the email_unread properties.
properties.set_status_unseen(
(properties.email_unread + total_unread_change).clamp(0, int.MAX)
);
}
if (complete_ids.size > 0)
email_complete(complete_ids);
......
......@@ -77,54 +77,62 @@ private class Geary.ImapEngine.EmailPrefetcher : Geary.BaseObject {
// emails should include PROPERTIES
private void schedule_prefetch(Gee.Collection<Geary.Email>? emails) {
if (emails == null || emails.size == 0)
return;
if (emails != null && emails.size > 0) {
this.prefetch_emails.add_all(emails);
debug("%s: scheduling %d emails for prefetching",
folder.to_string(), emails.size);
this.prefetch_emails.add_all(emails);
// only increment active state if not rescheduling
if (!this.prefetch_timer.is_running) {
this.active_sem.acquire();
}
// only increment active state if not rescheduling
if (!this.prefetch_timer.is_running) {
this.active_sem.acquire();
this.prefetch_timer.start();
}
this.prefetch_timer.start();
}
private async void do_prepare_all_local_async() {
Gee.List<Geary.Email>? list = null;
try {
debug("Listing all emails needing prefetching in %s...", folder.to_string());
list = yield folder.local_folder.list_email_by_id_async(null, int.MAX,
Geary.Email.Field.PROPERTIES, ImapDB.Folder.ListFlags.ONLY_INCOMPLETE, cancellable);
debug("Listed all emails needing prefetching in %s", folder.to_string());
} catch (Error err) {
debug("Error while list local emails for %s: %s", folder.to_string(), err.message);
list = yield this.folder.local_folder.list_email_by_id_async(
null, int.MAX,
Geary.Email.Field.PROPERTIES,
ImapDB.Folder.ListFlags.ONLY_INCOMPLETE,
this.cancellable
);
} catch (GLib.IOError.CANCELLED err) {
// all good
} catch (GLib.Error err) {
warning("%s: Error listing email on open: %s",
folder.to_string(), err.message);
}
debug("%s: Scheduling %d messages on open for prefetching",
this.folder.to_string(), list != null ? list.size : 0);
schedule_prefetch(list);
active_sem.blind_notify();
this.active_sem.blind_notify();
}
private async void do_prepare_new_async(Gee.Collection<Geary.EmailIdentifier> ids) {
Gee.List<Geary.Email>? list = null;
try {
debug("Listing new %d emails needing prefetching in %s...", ids.size, folder.to_string());
list = yield folder.local_folder.list_email_by_sparse_id_async(
list = yield this.folder.local_folder.list_email_by_sparse_id_async(
(Gee.Collection<ImapDB.EmailIdentifier>) ids,
Geary.Email.Field.PROPERTIES, ImapDB.Folder.ListFlags.ONLY_INCOMPLETE, cancellable);
debug("Listed new emails needing prefetching in %s", folder.to_string());
} catch (Error err) {
debug("Error while list local emails for %s: %s", folder.to_string(), err.message);
Geary.Email.Field.PROPERTIES,
ImapDB.Folder.ListFlags.ONLY_INCOMPLETE,
this.cancellable
);
} catch (GLib.IOError.CANCELLED err) {
// all good
} catch (GLib.Error err) {
warning("%s: Error listing email on open: %s",
folder.to_string(), err.message);
}
debug("%s: Scheduling %d new emails for prefetching",
this.folder.to_string(), list != null ? list.size : 0);
schedule_prefetch(list);
active_sem.blind_notify();
this.active_sem.blind_notify();
}
private async void do_prefetch_async() {
int token = Nonblocking.Mutex.INVALID_TOKEN;
try {
......
......@@ -530,10 +530,15 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Gee.Set<ImapDB.EmailIdentifier> inserted_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
Gee.Set<ImapDB.EmailIdentifier> locally_inserted_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
if (to_create.size > 0) {
Gee.Map<Email, bool>? created_or_merged = yield local_folder.create_or_merge_email_async(
to_create, cancellable);
// Don't update the unread count here, since it'll get
// updated once normalisation has finished anyway. See
// also Issue #213.
Gee.Map<Email, bool>? created_or_merged =
yield local_folder.create_or_merge_email_async(
to_create, false, cancellable
);
assert(created_or_merged != null);
// it's possible a large number of messages have come in, so process them in the
// background
yield Nonblocking.Concurrent.global.schedule_async(() => {
......@@ -1486,7 +1491,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Gee.List<Geary.Email>? list_remote = yield list_email_by_sparse_id_async(
local_map.keys,
Email.Field.FLAGS,
Geary.Folder.ListFlags.FORCE_UPDATE,
Folder.ListFlags.FORCE_UPDATE |
// Updating read/unread count here breaks the unread
// count, so don't do it. See issue #213.
Folder.ListFlags.NO_UNREAD_UPDATE,
cancellable
);
if (list_remote == null || list_remote.is_empty)
......
......@@ -18,6 +18,7 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
public Imap.MessageSet msg_set;
public Geary.Email.Field unfulfilled_fields;
public Geary.Email.Field required_fields;
public bool update_unread;
// OUT
public Gee.Set<Geary.EmailIdentifier> created_ids = new Gee.HashSet<Geary.EmailIdentifier>();
......@@ -26,12 +27,14 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
ImapDB.Folder local,
Imap.MessageSet msg_set,
Geary.Email.Field unfulfilled_fields,
Geary.Email.Field required_fields) {
Geary.Email.Field required_fields,
bool update_unread) {
this.remote = remote;
this.local = local;
this.msg_set = msg_set;
this.unfulfilled_fields = unfulfilled_fields;
this.required_fields = required_fields;
this.update_unread = update_unread;
}
public override async Object? execute_async(Cancellable? cancellable) throws Error {
......@@ -43,8 +46,12 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
return null;
// TODO: create_or_merge_email_async() should only write if something has changed
Gee.Map<Geary.Email, bool> created_or_merged = yield this.local.create_or_merge_email_async(
list, cancellable);
Gee.Map<Email, bool> created_or_merged =
yield this.local.create_or_merge_email_async(
list,
this.update_unread,
cancellable
);
for (int ctr = 0; ctr < list.size; ctr++) {
Geary.Email email = list[ctr];
......@@ -171,7 +178,8 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
this.owner.local_folder,
msg_set,
unfulfilled_fields,
required_fields
required_fields,
!this.flags.is_any_set(NO_UNREAD_UPDATE)
);
batch.add(remote_op);
}
......
......@@ -56,8 +56,12 @@ private class Geary.ImapEngine.CreateEmail : Geary.ImapEngine.SendReplayOperatio
if (created_id != null) {
// TODO: need to prevent gaps that may occur here
Geary.Email created = new Geary.Email(created_id);
Gee.Map<Geary.Email, bool> results = yield engine.local_folder.create_or_merge_email_async(
Geary.iterate<Geary.Email>(created).to_array_list(), cancellable);
Gee.Map<Geary.Email, bool> results =
yield this.engine.local_folder.create_or_merge_email_async(
Geary.iterate<Geary.Email>(created).to_array_list(),
true,
this.cancellable
);
if (results.size > 0) {
created_id = Collection.get_first<Geary.Email>(results.keys).id;
} else {
......
......@@ -62,6 +62,7 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation
// If returned in full, done
if (email.fields.fulfills(required_fields)) {
this.email = email;
this.remaining_fields = Email.Field.NONE;
return ReplayOperation.Status.COMPLETED;
}
......@@ -109,8 +110,8 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation
throw new EngineError.NOT_FOUND("Unable to fetch %s in %s", id.to_string(), engine.to_string());
Gee.Map<Geary.Email, bool> created_or_merged =
yield engine.local_folder.create_or_merge_email_async(
list, cancellable
yield this.engine.local_folder.create_or_merge_email_async(
list, true, this.cancellable
);
Geary.Email email = list[0];
......@@ -130,7 +131,12 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation
}
public override string describe_state() {
return "id=%s required_fields=%Xh remaining_fields=%Xh flags=%Xh".printf(id.to_string(),
required_fields, remaining_fields, flags);
return "id=%s required_fields=%Xh remaining_fields=%Xh flags=%Xh has_email=%s".printf(
this.id.to_string(),
this.required_fields,
this.remaining_fields,
this.flags,
(this.email == null).to_string()
);
}
}
......@@ -58,6 +58,21 @@ private class Geary.ImapEngine.MoveEmailCommit : Geary.ImapEngine.SendReplayOper
);
}
// The copy is implemented as follows:
// 1. Have an open connection to source folder
// 2. IMAP COPY from source to destination folder
// 3. IMAP STORE \Deleted
// 4. IMAP EXPUNGE
//
// The net result is that since the source folder will
// notify that the email has been marked as deleted
// then actually removed, the email will be flagged as
// deleted, and will need to be unflagged as such once
// it is noticed in the destination folder. This
// happens via the account synchroniser being notified
// that the destination has had its contents altered,
// so it goes off and checks.
Imap.MessageSet msg_set = iter.get();
Gee.Map<Imap.UID, Imap.UID>? map = yield remote.copy_email_async(
......@@ -65,7 +80,7 @@ private class Geary.ImapEngine.MoveEmailCommit : Geary.ImapEngine.SendReplayOper
);
if (map != null)
destination_uids.add_all(map.values);
yield remote.remove_email_async(msg_set.to_list(), null);
// completed successfully, remove from list in case of retry
......
......@@ -90,7 +90,9 @@ private class Geary.ImapEngine.ReplayAppend : Geary.ImapEngine.ReplayOperation {
// need to report both if it was created (not known before) and appended (which
// could mean created or simply a known email associated with this folder)
Gee.Map<Geary.Email, bool> created_or_merged =
yield this.owner.local_folder.create_or_merge_email_async(list, this.cancellable);
yield this.owner.local_folder.create_or_merge_email_async(
list, true, this.cancellable
);
foreach (Geary.Email email in created_or_merged.keys) {
// true means created
if (created_or_merged.get(email)) {
......
......@@ -17,7 +17,9 @@ class Geary.ImapDB.FolderTest : TestCase {
public FolderTest() {
base("Geary.ImapDB.FolderTest");
add_test("create_email", create_email);
add_test("create_read_email", create_read_email);
add_test("create_unread_email", create_unread_email);
add_test("create_no_unread_update", create_no_unread_update);
add_test("merge_email", merge_email);
add_test("merge_add_flags", merge_add_flags);
add_test("merge_remove_flags", merge_remove_flags);
......@@ -75,11 +77,12 @@ class Geary.ImapDB.FolderTest : TestCase {
this.tmp_dir = null;
}
public void create_email() throws GLib.Error {
public void create_read_email() throws GLib.Error {
Email mock = new_mock_remote_email(1, "test");
this.folder.create_or_merge_email_async.begin(
Collection.single(mock),
true,
null,
(obj, ret) => { async_complete(ret); }
);
......@@ -88,6 +91,45 @@ class Geary.ImapDB.FolderTest : TestCase {
assert_int(1, results.size);
assert(results.get(mock));
assert_int(0, this.folder.get_properties().email_unread);
}
public void create_unread_email() throws GLib.Error {
Email mock = new_mock_remote_email(
1, "test", new EmailFlags.with(EmailFlags.UNREAD)
);
this.folder.create_or_merge_email_async.begin(
Collection.single(mock),
true,
null,
(obj, ret) => { async_complete(ret); }
);
Gee.Map<Email,bool> results =
this.folder.create_or_merge_email_async.end(async_result());
assert_int(1, results.size);
assert(results.get(mock));
assert_int(1, this.folder.get_properties().email_unread);
}
public void create_no_unread_update() throws GLib.Error {
Email mock = new_mock_remote_email(
1, "test", new EmailFlags.with(EmailFlags.UNREAD)
);
this.folder.create_or_merge_email_async.begin(
Collection.single(mock),
false,
null,
(obj, ret) => { async_complete(ret); }
);
Gee.Map<Email,bool> results =
this.folder.create_or_merge_email_async.end(async_result());
assert_int(1, results.size);
assert(results.get(mock));
assert_int(0, this.folder.get_properties().email_unread);
}
public void merge_email() throws GLib.Error {
......@@ -107,6 +149,7 @@ class Geary.ImapDB.FolderTest : TestCase {
this.folder.create_or_merge_email_async.begin(
Collection.single(mock),
true,
null,
(obj, ret) => { async_complete(ret); }
);
......@@ -137,10 +180,7 @@ class Geary.ImapDB.FolderTest : TestCase {
}
public void merge_add_flags() throws GLib.Error {
// Note: Flags in the DB are expected to be Imap.MessageFlags,
// and flags passed in to ImapDB.Folder are expected to be
// Imap.EmailFlags
// Flags in the DB are expected to be Imap.MessageFlags
Email.Field fixture_fields = Email.Field.FLAGS;
Imap.MessageFlags fixture_flags =
new Imap.MessageFlags(Collection.single(Imap.MessageFlag.SEEN));
......@@ -155,13 +195,11 @@ class Geary.ImapDB.FolderTest : TestCase {
VALUES (1, 1, 1, 1);
""");
Imap.EmailFlags test_flags = Imap.EmailFlags.from_api_email_flags(
new EmailFlags.with(EmailFlags.UNREAD)
) ;
EmailFlags test_flags = new EmailFlags.with(EmailFlags.UNREAD);
Email test = new_mock_remote_email(1, null, test_flags);
this.folder.create_or_merge_email_async.begin(
Collection.single(test),
true,
null,
(obj, ret) => { async_complete(ret); }
);
......@@ -175,10 +213,7 @@ class Geary.ImapDB.FolderTest : TestCase {
}
public void merge_remove_flags() throws GLib.Error {
// Note: Flags in the DB are expected to be Imap.MessageFlags,
// and flags passed in to ImapDB.Folder are expected to be
// Imap.EmailFlags
// Flags in the DB are expected to be Imap.MessageFlags
Email.Field fixture_fields = Email.Field.FLAGS;
Imap.MessageFlags fixture_flags =
new Imap.MessageFlags(Gee.Collection.empty<Geary.Imap.MessageFlag>());
......@@ -193,12 +228,11 @@ class Geary.ImapDB.FolderTest : TestCase {
VALUES (1, 1, 1, 1);
""");
Imap.EmailFlags test_flags = Imap.EmailFlags.from_api_email_flags(
new EmailFlags()
) ;
EmailFlags test_flags = new EmailFlags();
Email test = new_mock_remote_email(1, null, test_flags);
this.folder.create_or_merge_email_async.begin(
Collection.single(test),
true,
null,
(obj, ret) => { async_complete(ret); }
);
......@@ -283,15 +317,17 @@ class Geary.ImapDB.FolderTest : TestCase {
private Email new_mock_remote_email(int64 uid,
string? subject = null,
EmailFlags? flags = null) {
Geary.EmailFlags? flags = null) {
Email mock = new Email(
new EmailIdentifier.no_message_id(new Imap.UID(uid))
);
if (subject != null) {
mock.set_message_subject(new RFC822.Subject(subject));
}
// Flags passed in to ImapDB.Folder are expected to be
// Imap.EmailFlags
if (flags != null) {
mock.set_flags(flags);
mock.set_flags(Imap.EmailFlags.from_api_email_flags(flags));
}
return mock;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment