[geary/wip/721828-undo] Further refinments
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/721828-undo] Further refinments
- Date: Wed, 31 Dec 2014 03:08:04 +0000 (UTC)
commit cb89941ab4973efeec659105b4c5ac2cd859da80
Author: Jim Nelson <jim yorba org>
Date: Tue Dec 30 19:07:13 2014 -0800
Further refinments
Fixes problems where linked/unlinked messages are not properly
re-linked during normalization when they're detected
src/client/application/geary-controller.vala | 2 +-
src/engine/imap-db/imap-db-folder.vala | 72 ++++++++++++--------
.../imap-engine/imap-engine-minimal-folder.vala | 8 ++-
.../imap-engine/imap-engine-revokable-move.vala | 13 ++++
.../replay-ops/imap-engine-move-email.vala | 9 +--
5 files changed, 67 insertions(+), 37 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 87e6124..192a351 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -2370,7 +2370,7 @@ public class GearyController : Geary.BaseObject {
Gtk.Action undo_action = GearyApplication.instance.get_action(ACTION_UNDO);
undo_action.sensitive = revokable != null && revokable.can_revoke;
- undo_action.tooltip = (revokable != null && description != null) ? description : "";
+ undo_action.tooltip = (revokable != null && description != null) ? description : _("Undo");
}
private void on_can_revoke_changed() {
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index d382c3f..178fba3 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -730,28 +730,56 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// EmailIdentifiers should contain valid UIDs *for this folder* for successful linking.
public async void link_multiple_emails_async(Gee.Collection<ImapDB.EmailIdentifier> ids,
Cancellable? cancellable) throws Error {
+ // store in hash set for closure, which potentially modifies the collection
+ Gee.HashSet<ImapDB.EmailIdentifier> id_set = traverse<ImapDB.EmailIdentifier>(ids).to_hash_set();
+
int unread_count = 0;
yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => {
// Can't use do_get_locations_for_ids, as that's keyed to this folder, which these
// emails don't belong to yet
- foreach (ImapDB.EmailIdentifier id in ids) {
- if (id.uid == null)
+ Gee.Iterator<ImapDB.EmailIdentifier> iter = id_set.iterator();
+ while (iter.next()) {
+ ImapDB.EmailIdentifier id = iter.get();
+ if (id.uid == null) {
+ iter.remove();
+
continue;
+ }
+ // If already present, don't insert
Db.Statement stmt = cx.prepare("""
+ SELECT id
+ FROM MessageLocationTable
+ WHERE message_id = ? AND folder_id = ? AND ordering = ?
+ """);
+ stmt.bind_rowid(0, id.message_id);
+ stmt.bind_rowid(1, folder_id);
+ stmt.bind_int64(2, id.uid.value);
+
+ Db.Result result = stmt.exec(cancellable);
+ if (!result.finished) {
+ // clear remove marker, if set
+ do_mark_unmark_removed(cx, iterate<Imap.UID>(id.uid).to_array_list(), false,
+ cancellable);
+
+ continue;
+ }
+
+ stmt = cx.prepare("""
INSERT INTO MessageLocationTable
- (message_id, folder_id, ordering)
- VALUES (?, ?, ?)
+ (message_id, folder_id, ordering, remove_marker)
+ VALUES (?, ?, ?, ?)
""");
stmt.bind_rowid(0, id.message_id);
stmt.bind_rowid(1, folder_id);
stmt.bind_int64(2, id.uid.value);
+ stmt.bind_bool(3, false);
stmt.exec(cancellable);
}
- // Now with messages linked to folder, update unread count
- unread_count = do_get_unread_count_for_ids(cx, ids, cancellable);
+ // Now with messages linked to folder, update unread count with ids that were linked
+ unread_count = do_get_unread_count_for_ids(cx, id_set, cancellable);
do_add_to_unread_count(cx, unread_count, cancellable);
return Db.TransactionOutcome.COMMIT;
@@ -1187,14 +1215,14 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
LocationIdentifier? location = null;
// See if it already exists; first by UID (which is only guaranteed to
// be unique in a folder, not account-wide)
- if (email_id.uid != null)
+ if (email_id.uid != null) {
location = do_get_location_for_uid(cx, email_id.uid, ListFlags.INCLUDE_MARKED_FOR_REMOVE,
cancellable);
-
- if (location != null) {
- associated = true;
-
- return location;
+ if (location != null) {
+ associated = true;
+
+ return location;
+ }
}
// if fields not present, then no duplicate can reliably be found
@@ -1226,32 +1254,18 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_int64(1, rfc822_size);
Db.Result results = stmt.exec(cancellable);
- // no duplicates found
if (results.finished)
return null;
+ // if found, then consider it to be a duplicate that can be associated with this folder
int64 message_id = results.rowid_at(0);
if (results.next(cancellable)) {
debug("Warning: multiple messages with the same internaldate (%s) and size (%s) in %s",
internaldate, rfc822_size.to_string(), to_string());
}
- Db.Statement search_stmt = cx.prepare(
- "SELECT ordering, remove_marker FROM MessageLocationTable WHERE message_id=? AND folder_id=?");
- search_stmt.bind_rowid(0, message_id);
- search_stmt.bind_rowid(1, folder_id);
-
- Db.Result search_results = search_stmt.exec(cancellable);
- if (!search_results.finished) {
- associated = true;
- location = new LocationIdentifier(message_id, new Imap.UID(search_results.int64_at(0)),
- search_results.bool_at(1));
- } else {
- assert(email_id.uid != null);
- location = new LocationIdentifier(message_id, email_id.uid, false);
- }
-
- return location;
+ assert(email_id.uid != null);
+ return new LocationIdentifier(message_id, email_id.uid, false);
}
// Note: does NOT check if message is already associated with thie folder
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 9089df6..d2a3e7a 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -223,11 +223,15 @@ private class Geary.ImapEngine.MinimalFolder : Geary.AbstractFolder, Geary.Folde
// if UIDNEXT has changed, that indicates messages have been appended (and possibly removed)
int64 uidnext_diff = remote_properties.uid_next.value - local_properties.uid_next.value;
- int local_message_count = (local_properties.select_examine_messages >= 0)
- ? local_properties.select_examine_messages : 0;
+ int local_message_count = yield local_folder.get_email_count_async(
+ ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
+
int remote_message_count = (remote_properties.select_examine_messages >= 0)
? remote_properties.select_examine_messages : 0;
+ debug("%s: local_message_count=%d remote_message_count=%d", to_string(),
+ local_message_count, remote_message_count);
+
// if UIDNEXT is the same as last time AND the total count of email is the same, then
// nothing has been added or removed
if (!is_dirty && uidnext_diff == 0 && local_message_count == remote_message_count) {
diff --git a/src/engine/imap-engine/imap-engine-revokable-move.vala
b/src/engine/imap-engine/imap-engine-revokable-move.vala
index 64e9a5e..d1a58ee 100644
--- a/src/engine/imap-engine/imap-engine-revokable-move.vala
+++ b/src/engine/imap-engine/imap-engine-revokable-move.vala
@@ -63,6 +63,18 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
if (can_revoke) {
yield dest_folder.revoke_move_async(destination_ids, original_source, cancellable);
can_revoke = false;
+
+ // there's not a super-reliable way to wait until the delete of the message in this
+ // folder has completed; could wait for the UID to be reported deleted, but it's
+ // possible in IMAP to delete a message not present in folder and not get an error
+ // (but also not be notified of its removal), and so waiting is a bad idea. Don't
+ // want to close the folder immediately because that leaves the local remove_marker
+ // in place and the folder is treated as "dirty", causing a full renormalize to
+ // occur when next opened, which is expensive. So: just wait a bit and give the
+ // server a chance to report the message is gone, leaving the db in a good state.
+ // Do this *after* marking can_revoke=false so client is notified that this object
+ // is now worthless.
+ yield Scheduler.sleep_async(5);
}
} finally {
// note that the Cancellable is not used
@@ -101,6 +113,7 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
// convert generic identifiers to UIDs
Gee.HashSet<Imap.UID> removed_uids = traverse<EmailIdentifier>(ids)
.cast_object<ImapDB.EmailIdentifier>()
+ .filter(id => id.uid == null)
.map<Imap.UID>(id => id.uid)
.to_hash_set();
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala
b/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala
index aa89846..4a6c61b 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala
@@ -50,7 +50,7 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation
engine.notify_email_removed(moved_ids);
- engine.notify_email_count_changed(Numeric.int_floor(original_count - to_move.size, 0),
+ engine.notify_email_count_changed(Numeric.int_floor(original_count - moved_ids.size, 0),
Geary.Folder.CountChangeReason.REMOVED);
// Report to the local destination Folder that new messages are predicted to arrive
@@ -98,6 +98,7 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation
foreach (Imap.MessageSet msg_set in msg_sets) {
Gee.Map<Imap.UID, Imap.UID>? copyuids = yield engine.remote_folder.copy_email_async(msg_set,
destination, null);
+ yield engine.remote_folder.remove_email_async(msg_set, null);
// convert returned COPYUID response into EmailIdentifiers for the destination folder
if (copyuids != null && copyuids.size > 0) {
@@ -111,21 +112,19 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation
}
}
}
-
- yield engine.remote_folder.remove_email_async(msg_set, null);
}
if (acc_ids.size > 0) {
// link the destination id's to the local folder, which saves a little trouble when
// normalizing with the remote as well as makes revoking them work properly
- MinimalFolder? dest_folder = (yield engine.account.fetch_folder_async(destination, cancellable))
+ MinimalFolder? dest_folder = (yield engine.account.fetch_folder_async(destination))
as MinimalFolder;
if (dest_folder != null) {
try {
// Note that this doesn't require dest_folder be open because we're going straight
// to the database...dest_folder will announce to its subscribers changes when
// normalization/notification occurs via IMAP
- yield dest_folder.local_folder.link_multiple_emails_async(acc_ids, cancellable);
+ yield dest_folder.local_folder.link_multiple_emails_async(acc_ids, null);
destination_ids = acc_ids;
} catch (Error err) {
debug("Unable to link moved emails to new folder: %s", err.message);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
