[geary/wip/778276-better-flag-updates: 15/17] Move impl for ReplayAppend and ReplayRemoval actually into the ops.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/778276-better-flag-updates: 15/17] Move impl for ReplayAppend and ReplayRemoval actually into the ops.
- Date: Tue, 5 Dec 2017 01:58:25 +0000 (UTC)
commit e1e78d348123c237b9f7d9fefc552050df10b291
Author: Michael James Gratton <mike vee net>
Date: Tue Nov 28 10:29:28 2017 +1100
Move impl for ReplayAppend and ReplayRemoval actually into the ops.
This de-clutters MinimalFolder and ensures they really only can be called
from the replay ops, as intended.
.../imap-engine/imap-engine-minimal-folder.vala | 216 +++-----------------
.../replay-ops/imap-engine-replay-append.vala | 94 ++++++++-
.../replay-ops/imap-engine-replay-removal.vala | 120 ++++++++++-
3 files changed, 227 insertions(+), 203 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index f0bb6e1..ec2d319 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -55,7 +55,9 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
internal Imap.Folder? remote_folder { get; protected set; default = null; }
internal EmailPrefetcher email_prefetcher { get; private set; }
internal EmailFlagWatcher email_flag_watcher;
-
+ internal int remote_count { get; private set; default = -1; }
+ internal ReplayQueue replay_queue { get; private set; }
+
private weak GenericAccount _account;
private Geary.AggregatedFolderProperties _properties = new Geary.AggregatedFolderProperties(
false, false);
@@ -68,8 +70,6 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
private Nonblocking.ReportingSemaphore<bool> remote_semaphore =
new Nonblocking.ReportingSemaphore<bool>(false);
private Nonblocking.Semaphore closed_semaphore = new Nonblocking.Semaphore();
- private ReplayQueue replay_queue;
- private int remote_count = -1;
private Nonblocking.Mutex open_mutex = new Nonblocking.Mutex();
private Nonblocking.Mutex close_mutex = new Nonblocking.Mutex();
private TimeoutManager refresh_unseen_timer;
@@ -1074,95 +1074,20 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Gee.List<Imap.SequenceNumber> positions = new Gee.ArrayList<Imap.SequenceNumber>();
for (int pos = remote_count + 1; pos <= reported_remote_count; pos++)
positions.add(new Imap.SequenceNumber(pos));
-
+
// store the remote count NOW, as further appended messages could arrive before the
// ReplayAppend executes
- remote_count = reported_remote_count;
-
- if (positions.size > 0)
- replay_queue.schedule_server_notification(new ReplayAppend(this, reported_remote_count,
positions));
- }
-
- // Need to prefetch at least an EmailIdentifier (and duplicate detection fields) to create a
- // normalized placeholder in the local database of the message, so all positions are
- // properly relative to the end of the message list; once this is done, notify user of new
- // messages. If duplicates, create_email_async() will fall through to an updated merge,
- // which is exactly what we want.
- //
- // This MUST only be called from ReplayAppend.
- internal async void do_replay_appended_messages(int reported_remote_count,
- Gee.List<Imap.SequenceNumber> remote_positions) {
- StringBuilder positions_builder = new StringBuilder("( ");
- foreach (Imap.SequenceNumber remote_position in remote_positions)
- positions_builder.append_printf("%s ", remote_position.to_string());
- positions_builder.append(")");
-
- debug("%s do_replay_appended_message: current remote_count=%d reported_remote_count=%d
remote_positions=%s",
- to_string(), remote_count, reported_remote_count, positions_builder.str);
-
- if (remote_positions.size == 0)
- return;
-
- Gee.HashSet<Geary.EmailIdentifier> created = new Gee.HashSet<Geary.EmailIdentifier>();
- Gee.HashSet<Geary.EmailIdentifier> appended = new Gee.HashSet<Geary.EmailIdentifier>();
- try {
- Gee.List<Imap.MessageSet> msg_sets = Imap.MessageSet.sparse(remote_positions);
- foreach (Imap.MessageSet msg_set in msg_sets) {
- Gee.List<Geary.Email>? list = yield remote_folder.list_email_async(msg_set,
- ImapDB.Folder.REQUIRED_FIELDS, null);
- if (list != null && list.size > 0) {
- debug("%s do_replay_appended_message: %d new messages in %s", to_string(),
- list.size, msg_set.to_string());
-
- // 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 local_folder.create_or_merge_email_async(list, null);
- foreach (Geary.Email email in created_or_merged.keys) {
- // true means created
- if (created_or_merged.get(email)) {
- debug("%s do_replay_appended_message: appended email ID %s added",
- to_string(), email.id.to_string());
-
- created.add(email.id);
- } else {
- debug("%s do_replay_appended_message: appended email ID %s associated",
- to_string(), email.id.to_string());
- }
-
- appended.add(email.id);
- }
- } else {
- debug("%s do_replay_appended_message: no new messages in %s", to_string(),
- msg_set.to_string());
- }
- }
- } catch (Error err) {
- debug("%s do_replay_appended_message: Unable to process: %s",
- to_string(), err.message);
- }
-
- // store the reported count, *not* the current count (which is updated outside the of
- // the queue) to ensure that updates happen serially and reflect committed local changes
- try {
- yield local_folder.update_remote_selected_message_count(reported_remote_count, null);
- } catch (Error err) {
- debug("%s do_replay_appended_message: Unable to save appended remote count %d: %s",
- to_string(), reported_remote_count, err.message);
+ this.remote_count = reported_remote_count;
+
+ if (positions.size > 0) {
+ ReplayAppend op = new ReplayAppend(this, reported_remote_count, positions);
+ op.email_appended.connect(notify_email_appended);
+ op.email_locally_appended.connect(notify_email_locally_appended);
+ op.email_count_changed.connect(notify_email_count_changed);
+ this.replay_queue.schedule_server_notification(op);
}
-
- if (appended.size > 0)
- notify_email_appended(appended);
-
- if (created.size > 0)
- notify_email_locally_appended(created);
-
- notify_email_count_changed(reported_remote_count, CountChangeReason.APPENDED);
-
- debug("%s do_replay_appended_message: completed, current remote_count=%d reported_remote_count=%d",
- to_string(), remote_count, reported_remote_count);
}
-
+
private void on_remote_removed(Imap.SequenceNumber position, int reported_remote_count) {
debug("%s on_remote_removed: remote_count=%d position=%s reported_remote_count=%d", to_string(),
remote_count, position.to_string(), reported_remote_count);
@@ -1181,109 +1106,15 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// remote_count is *not* updated, which is why it's safe to do that here without worry.
// similarly, signals are only fired here if marked, so the same EmailIdentifier isn't
// reported twice
- remote_count = reported_remote_count;
-
- replay_queue.schedule_server_notification(new ReplayRemoval(this, reported_remote_count, position));
- }
-
- // This MUST only be called from ReplayRemoval.
- internal async void do_replay_removed_message(int reported_remote_count, Imap.SequenceNumber
remote_position) {
- debug("%s do_replay_removed_message: current remote_count=%d remote_position=%s
reported_remote_count=%d",
- to_string(), remote_count, remote_position.value.to_string(), reported_remote_count);
-
- if (!remote_position.is_valid()) {
- debug("%s do_replay_removed_message: ignoring, invalid remote position or count",
- to_string());
-
- return;
- }
-
- int local_count = -1;
- int64 local_position = -1;
-
- ImapDB.EmailIdentifier? owned_id = null;
- try {
- // need total count, including those marked for removal, to accurately calculate position
- // from server's point of view, not client's
- local_count = yield local_folder.get_email_count_async(
- ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE, null);
- local_position = remote_position.value - (reported_remote_count + 1 - local_count);
-
- // zero or negative means the message exists beyond the local vector's range, so
- // nothing to do there
- if (local_position > 0) {
- debug("%s do_replay_removed_message: local_count=%d local_position=%s", to_string(),
- local_count, local_position.to_string());
-
- owned_id = yield local_folder.get_id_at_async(local_position, null);
- } else {
- debug("%s do_replay_removed_message: message not stored locally (local_count=%d
local_position=%s)",
- to_string(), local_count, local_position.to_string());
- }
- } catch (Error err) {
- debug("%s do_replay_removed_message: unable to determine ID of removed message %s: %s",
- to_string(), remote_position.to_string(), err.message);
- }
-
- bool marked = false;
- if (owned_id != null) {
- debug("%s do_replay_removed_message: detaching from local store Email ID %s", to_string(),
- owned_id.to_string());
- try {
- // Reflect change in the local store and notify subscribers
- yield local_folder.detach_single_email_async(owned_id, out marked, null);
- } catch (Error err) {
- debug("%s do_replay_removed_message: unable to remove message #%s: %s", to_string(),
- remote_position.to_string(), err.message);
- }
-
- // Notify queued replay operations that the email has been removed (by EmailIdentifier)
- replay_queue.notify_remote_removed_ids(
- Geary.iterate<ImapDB.EmailIdentifier>(owned_id).to_array_list());
- } else {
- debug("%s do_replay_removed_message: remote_position=%ld unknown in local store "
- + "(reported_remote_count=%d local_position=%ld local_count=%d)",
- to_string(), remote_position.value, reported_remote_count, local_position, local_count);
- }
-
- // for debugging
- int new_local_count = -1;
- try {
- new_local_count = yield local_folder.get_email_count_async(
- ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE, null);
- } catch (Error err) {
- debug("%s do_replay_removed_message: error fetching new local count: %s", to_string(),
- err.message);
- }
-
- // as with on_remote_appended(), only update in local store inside a queue operation, to
- // ensure serial commits
- try {
- yield local_folder.update_remote_selected_message_count(reported_remote_count, null);
- } catch (Error err) {
- debug("%s do_replay_removed_message: unable to save removed remote count: %s", to_string(),
- err.message);
- }
-
- // notify of change ... use "marked-email-removed" for marked email to allow internal code
- // to be notified when a removed email is "really" removed
- if (owned_id != null) {
- Gee.List<EmailIdentifier> removed =
Geary.iterate<Geary.EmailIdentifier>(owned_id).to_array_list();
- if (!marked)
- notify_email_removed(removed);
- else
- marked_email_removed(removed);
- }
-
- if (!marked)
- notify_email_count_changed(reported_remote_count, CountChangeReason.REMOVED);
-
- debug("%s do_replay_remove_message: completed, current remote_count=%d "
- + "(reported_remote_count=%d local_count=%d starting local_count=%d remote_position=%ld
local_position=%ld marked=%s)",
- to_string(), remote_count, reported_remote_count, new_local_count, local_count,
remote_position.value,
- local_position, marked.to_string());
+ this.remote_count = reported_remote_count;
+
+ ReplayRemoval op = new ReplayRemoval(this, reported_remote_count, position);
+ op.email_removed.connect(notify_email_removed);
+ op.marked_email_removed.connect(notify_marked_email_removed);
+ op.email_count_changed.connect(notify_email_count_changed);
+ this.replay_queue.schedule_server_notification(op);
}
-
+
private void on_remote_disconnected(Imap.ClientSession.DisconnectReason reason) {
debug("on_remote_disconnected: reason=%s", reason.to_string());
@@ -1592,6 +1423,11 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
));
}
+ /** Fires a {@link marked_email_removed}} signal for this folder. */
+ protected virtual void notify_marked_email_removed(Gee.Collection<Geary.EmailIdentifier> removed) {
+ marked_email_removed(removed);
+ }
+
public override string to_string() {
return "%s (open_count=%d remote_opened=%s)".printf(base.to_string(), open_count,
remote_opened.to_string());
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 7a271e5..11ab3f7 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
@@ -5,10 +5,16 @@
*/
private class Geary.ImapEngine.ReplayAppend : Geary.ImapEngine.ReplayOperation {
+
private MinimalFolder owner;
private int remote_count;
private Gee.List<Imap.SequenceNumber> positions;
-
+
+ public signal void email_appended(Gee.Collection<Geary.EmailIdentifier> ids);
+ public signal void email_locally_appended(Gee.Collection<Geary.EmailIdentifier> ids);
+ public signal void email_count_changed(int count, Folder.CountChangeReason reason);
+
+
public ReplayAppend(MinimalFolder owner, int remote_count, Gee.List<Imap.SequenceNumber> positions) {
// IGNORE remote errors because the reconnect will re-normalize the folder, making this
// append moot
@@ -51,16 +57,90 @@ private class Geary.ImapEngine.ReplayAppend : Geary.ImapEngine.ReplayOperation {
public override async void backout_local_async() throws Error {
}
-
+
public override async ReplayOperation.Status replay_remote_async() {
- if (positions.size > 0)
- yield owner.do_replay_appended_messages(remote_count, positions);
-
+ if (this.positions.size > 0)
+ yield do_replay_appended_messages();
+
return ReplayOperation.Status.COMPLETED;
}
-
+
public override string describe_state() {
return "remote_count=%d positions.size=%d".printf(remote_count, positions.size);
}
-}
+ // Need to prefetch at least an EmailIdentifier (and duplicate detection fields) to create a
+ // normalized placeholder in the local database of the message, so all positions are
+ // properly relative to the end of the message list; once this is done, notify user of new
+ // messages. If duplicates, create_email_async() will fall through to an updated merge,
+ // which is exactly what we want.
+ private async void do_replay_appended_messages() {
+ StringBuilder positions_builder = new StringBuilder("( ");
+ foreach (Imap.SequenceNumber remote_position in this.positions)
+ positions_builder.append_printf("%s ", remote_position.to_string());
+ positions_builder.append(")");
+
+ debug("%s do_replay_appended_message: current remote_count=%d this.remote_count=%d
this.positions=%s",
+ to_string(), remote_count, this.remote_count, positions_builder.str);
+
+ Gee.HashSet<Geary.EmailIdentifier> created = new Gee.HashSet<Geary.EmailIdentifier>();
+ Gee.HashSet<Geary.EmailIdentifier> appended = new Gee.HashSet<Geary.EmailIdentifier>();
+ try {
+ Gee.List<Imap.MessageSet> msg_sets = Imap.MessageSet.sparse(this.positions);
+ foreach (Imap.MessageSet msg_set in msg_sets) {
+ Gee.List<Geary.Email>? list = yield this.owner.remote_folder.list_email_async(msg_set,
+ ImapDB.Folder.REQUIRED_FIELDS, null);
+ if (list != null && list.size > 0) {
+ debug("%s do_replay_appended_message: %d new messages in %s", to_string(),
+ list.size, msg_set.to_string());
+
+ // 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, null);
+ foreach (Geary.Email email in created_or_merged.keys) {
+ // true means created
+ if (created_or_merged.get(email)) {
+ debug("%s do_replay_appended_message: appended email ID %s added",
+ to_string(), email.id.to_string());
+
+ created.add(email.id);
+ } else {
+ debug("%s do_replay_appended_message: appended email ID %s associated",
+ to_string(), email.id.to_string());
+ }
+
+ appended.add(email.id);
+ }
+ } else {
+ debug("%s do_replay_appended_message: no new messages in %s", to_string(),
+ msg_set.to_string());
+ }
+ }
+ } catch (Error err) {
+ debug("%s do_replay_appended_message: Unable to process: %s",
+ to_string(), err.message);
+ }
+
+ // store the reported count, *not* the current count (which is updated outside the of
+ // the queue) to ensure that updates happen serially and reflect committed local changes
+ try {
+ yield this.owner.local_folder.update_remote_selected_message_count(this.remote_count, null);
+ } catch (Error err) {
+ debug("%s do_replay_appended_message: Unable to save appended remote count %d: %s",
+ to_string(), this.remote_count, err.message);
+ }
+
+ if (appended.size > 0)
+ email_appended(appended);
+
+ if (created.size > 0)
+ email_locally_appended(created);
+
+ email_count_changed(this.remote_count, Folder.CountChangeReason.APPENDED);
+
+ debug("%s do_replay_appended_message: completed, current remote_count=%d this.remote_count=%d",
+ to_string(), remote_count, this.remote_count);
+ }
+
+}
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
b/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
index 0f5d6a9..55891a3 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-replay-removal.vala
@@ -5,10 +5,16 @@
*/
private class Geary.ImapEngine.ReplayRemoval : Geary.ImapEngine.ReplayOperation {
+
private MinimalFolder owner;
private int remote_count;
private Imap.SequenceNumber position;
-
+
+ public signal void email_removed(Gee.Collection<Geary.EmailIdentifier> ids);
+ public signal void marked_email_removed(Gee.Collection<Geary.EmailIdentifier> ids);
+ public signal void email_count_changed(int count, Folder.CountChangeReason reason);
+
+
public ReplayRemoval(MinimalFolder owner, int remote_count, Imap.SequenceNumber position) {
// remote error will cause folder to reconnect and re-normalize, making this remove moot
base ("Removal", Scope.LOCAL_AND_REMOTE, OnError.IGNORE);
@@ -39,15 +45,117 @@ private class Geary.ImapEngine.ReplayRemoval : Geary.ImapEngine.ReplayOperation
public override async void backout_local_async() throws Error {
}
-
+
public override async ReplayOperation.Status replay_remote_async() throws Error {
- yield owner.do_replay_removed_message(remote_count, position);
-
+ debug("%s: ReplayRemoval current remote_count=%d this.position=%s reported_remote_count=%d",
+ this.owner.to_string(), this.owner.remote_count,
+ this.position.value.to_string(), this.remote_count);
+
+ if (this.position.is_valid()) {
+ yield do_replay_removed_message();
+ } else {
+ debug("%s do_replay_removed_message: ignoring, invalid remote position or count",
+ to_string());
+ }
return ReplayOperation.Status.COMPLETED;
}
-
+
public override string describe_state() {
return "position=%s".printf(position.to_string());
}
-}
+ private async void do_replay_removed_message() {
+ int local_count = -1;
+ int64 local_position = -1;
+
+ ImapDB.EmailIdentifier? owned_id = null;
+ try {
+ // need total count, including those marked for removal,
+ // to accurately calculate position from server's point of
+ // view, not client's. The extra 1 taken off is due to the
+ // remote count already being decremented in MinimalFolder
+ // when this op was queued.
+ local_count = yield this.owner.local_folder.get_email_count_async(
+ ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE, null);
+ local_position = this.position.value - (this.remote_count + 1 - local_count);
+
+ // zero or negative means the message exists beyond the local vector's range, so
+ // nothing to do there
+ if (local_position > 0) {
+ debug("%s do_replay_removed_message: local_count=%d local_position=%s", to_string(),
+ local_count, local_position.to_string());
+
+ owned_id = yield this.owner.local_folder.get_id_at_async(local_position, null);
+ } else {
+ debug("%s do_replay_removed_message: message not stored locally (local_count=%d
local_position=%s)",
+ to_string(), local_count, local_position.to_string());
+ }
+ } catch (Error err) {
+ debug("%s do_replay_removed_message: unable to determine ID of removed message %s: %s",
+ to_string(), this.position.to_string(), err.message);
+ }
+
+ bool marked = false;
+ if (owned_id != null) {
+ debug("%s do_replay_removed_message: detaching from local store Email ID %s", to_string(),
+ owned_id.to_string());
+ try {
+ // Reflect change in the local store and notify subscribers
+ yield this.owner.local_folder.detach_single_email_async(owned_id, out marked, null);
+ } catch (Error err) {
+ debug("%s do_replay_removed_message: unable to remove message #%s: %s", to_string(),
+ this.position.to_string(), err.message);
+ }
+
+ // Notify queued replay operations that the email has been removed (by EmailIdentifier)
+ this.owner.replay_queue.notify_remote_removed_ids(
+ Geary.iterate<ImapDB.EmailIdentifier>(owned_id).to_array_list());
+ } else {
+ debug("%s do_replay_removed_message: this.position=%ld unknown in local store "
+ + "(this.remote_count=%d local_position=%ld local_count=%d)",
+ to_string(), this.position.value, this.remote_count, local_position, local_count);
+ }
+
+ // for debugging
+ int new_local_count = -1;
+ try {
+ new_local_count = yield this.owner.local_folder.get_email_count_async(
+ ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE, null);
+ } catch (Error err) {
+ debug("%s do_replay_removed_message: error fetching new local count: %s", to_string(),
+ err.message);
+ }
+
+ // as with on_remote_appended(), only update in local store inside a queue operation, to
+ // ensure serial commits
+ try {
+ yield this.owner.local_folder.update_remote_selected_message_count(this.remote_count, null);
+ } catch (Error err) {
+ debug("%s do_replay_removed_message: unable to save removed remote count: %s", to_string(),
+ err.message);
+ }
+
+ // notify of change ... use "marked-email-removed" for marked email to allow internal code
+ // to be notified when a removed email is "really" removed
+ if (owned_id != null) {
+ Gee.List<EmailIdentifier> removed =
Geary.iterate<Geary.EmailIdentifier>(owned_id).to_array_list();
+ if (!marked)
+ email_removed(removed);
+ else
+ marked_email_removed(removed);
+ }
+
+ if (!marked) {
+ this.owner.replay_notify_email_count_changed(
+ this.remote_count, Folder.CountChangeReason.REMOVED
+ );
+ }
+
+ debug("%s ReplayRemoval: completed, current remote_count=%d "
+ + "(this.remote_count=%d local_count=%d starting local_count=%d this.position=%ld
local_position=%ld marked=%s)",
+ this.owner.to_string(), this.owner.remote_count,
+ this.remote_count, new_local_count, local_count,
+ this.position.value, local_position, marked.to_string());
+ }
+
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
