diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index 227379f7..f8c0cea2 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -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; } diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index 53f8c5fb..8e22b5cf 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -285,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 create_or_merge_email_async(Gee.Collection emails, - Cancellable? cancellable) throws Error { + public async Gee.Map + create_or_merge_email_async(Gee.Collection emails, + bool update_totals, + GLib.Cancellable? cancellable) + throws GLib.Error { Gee.HashMap results = new Gee.HashMap(); Gee.ArrayList list = traverse(emails).to_array_list(); @@ -316,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); diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala b/src/engine/imap-engine/imap-engine-minimal-folder.vala index e76c1987..d5827733 100644 --- a/src/engine/imap-engine/imap-engine-minimal-folder.vala +++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala @@ -530,10 +530,15 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport Gee.Set inserted_ids = new Gee.HashSet(); Gee.Set locally_inserted_ids = new Gee.HashSet(); if (to_create.size > 0) { - Gee.Map? 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? 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? 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) diff --git a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala index 8adf79e2..16ef683d 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala @@ -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 created_ids = new Gee.HashSet(); @@ -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 created_or_merged = yield this.local.create_or_merge_email_async( - list, cancellable); + Gee.Map 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); } diff --git a/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala index 9cd5a89a..9062c235 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala @@ -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 results = yield engine.local_folder.create_or_merge_email_async( - Geary.iterate(created).to_array_list(), cancellable); + Gee.Map results = + yield this.engine.local_folder.create_or_merge_email_async( + Geary.iterate(created).to_array_list(), + true, + this.cancellable + ); if (results.size > 0) { created_id = Collection.get_first(results.keys).id; } else { diff --git a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala index e1a88844..5ecdc200 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala @@ -110,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 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]; diff --git a/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala b/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala index 9a5f9165..1908c64f 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-replay-append.vala @@ -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 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)) { diff --git a/test/engine/imap-db/imap-db-folder-test.vala b/test/engine/imap-db/imap-db-folder-test.vala index abb3fefd..ce32de7c 100644 --- a/test/engine/imap-db/imap-db-folder-test.vala +++ b/test/engine/imap-db/imap-db-folder-test.vala @@ -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 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 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()); @@ -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; }