Improvements and bug fixes while working on #7512

Working on Outlook.com identified some small bugs that affect
all services.  This patch is a culmination of that work.
This commit is contained in:
Jim Nelson 2013-09-23 17:58:49 -07:00
parent 1b270b8c82
commit 3cd37b9fed
9 changed files with 120 additions and 44 deletions

View file

@ -206,12 +206,12 @@ engine/imap-engine/outlook/imap-engine-outlook-folder.vala
engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala
engine/imap-engine/replay-ops/imap-engine-copy-email.vala
engine/imap-engine/replay-ops/imap-engine-create-email.vala
engine/imap-engine/replay-ops/imap-engine-expunge-email.vala
engine/imap-engine/replay-ops/imap-engine-fetch-email.vala
engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala
engine/imap-engine/replay-ops/imap-engine-list-email-by-sparse-id.vala
engine/imap-engine/replay-ops/imap-engine-mark-email.vala
engine/imap-engine/replay-ops/imap-engine-move-email.vala
engine/imap-engine/replay-ops/imap-engine-remove-email.vala
engine/imap-engine/replay-ops/imap-engine-replay-append.vala
engine/imap-engine/replay-ops/imap-engine-replay-disconnect.vala
engine/imap-engine/replay-ops/imap-engine-replay-removal.vala

View file

@ -730,9 +730,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
"DELETE FROM MessageLocationTable WHERE folder_id=?");
stmt.bind_rowid(0, folder_id);
stmt.exec(cancellable);
Db.Statement update_stmt = cx.prepare(
"UPDATE FolderTable SET unread_count = 0 WHERE id=?");
update_stmt.bind_rowid(0, folder_id);
update_stmt.exec(cancellable);
return Db.TransactionOutcome.COMMIT;
@ -942,6 +945,30 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return count;
}
public async Gee.Set<Imap.UID>? get_marked_uids_async(Cancellable? cancellable) throws Error {
Gee.Set<Imap.UID> uids = new Gee.HashSet<Imap.UID>();
yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
Db.Statement stmt = cx.prepare("""
SELECT ordering
FROM MessageLocationTable
WHERE folder_id=? AND remove_marker<>?
""");
stmt.bind_rowid(0, folder_id);
stmt.bind_bool(1, false);
Db.Result results = stmt.exec(cancellable);
while (!results.finished) {
uids.add(new Imap.UID(results.int64_at(0)));
results.next(cancellable);
}
return Db.TransactionOutcome.DONE;
}, cancellable);
return uids.size > 0 ? uids : null;
}
// Clears all remove markers from the folder
public async void clear_remove_markers_async(Cancellable? cancellable) throws Error {
yield db.exec_transaction_async(Db.TransactionType.WO, (cx) => {

View file

@ -160,11 +160,11 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
// if any messages are still marked for removal from last time, that means the EXPUNGE
// never arrived from the server, in which case the folder is "dirty" and needs a full
// normalization
int remove_markers = yield local_folder.get_marked_for_remove_count_async(cancellable);
bool is_dirty = (remove_markers != 0);
Gee.Set<Imap.UID>? marked_uids = yield local_folder.get_marked_uids_async(cancellable);
bool is_dirty = (marked_uids != null && marked_uids.size > 0);
if (is_dirty)
debug("%s: %d remove markers found, folder is dirty", to_string(), remove_markers);
debug("%s: %d remove markers found, folder is dirty", to_string(), marked_uids.size);
// 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;
@ -253,6 +253,15 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
else
inserted_uids.add(remote_uid);
}
// the UIDs marked for removal are going to be re-inserted into the vector once they're
// cleared, so add them here as well
if (marked_uids != null) {
foreach (Imap.UID uid in marked_uids) {
if (!appended_uids.contains(uid))
inserted_uids.add(uid);
}
}
}, cancellable);
debug("%s: changes since last seen: removed=%d appended=%d inserted=%d", to_string(),
@ -563,7 +572,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
}
// NOTE: This bypasses open_count and forces the Folder closed.
private async void close_internal_async(Folder.CloseReason local_reason, Folder.CloseReason remote_reason,
internal async void close_internal_async(Folder.CloseReason local_reason, Folder.CloseReason remote_reason,
Cancellable? cancellable) {
// force closed
open_count = 0;
@ -891,27 +900,10 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
private void on_remote_disconnected(Imap.ClientSession.DisconnectReason reason) {
debug("on_remote_disconnected: reason=%s", reason.to_string());
replay_queue.schedule(new ReplayDisconnect(this, reason));
}
internal async void do_replay_remote_disconnected(Imap.ClientSession.DisconnectReason reason) {
debug("do_replay_remote_disconnected reason=%s", reason.to_string());
Geary.Folder.CloseReason folder_reason = reason.is_error()
? Geary.Folder.CloseReason.REMOTE_ERROR : Geary.Folder.CloseReason.REMOTE_CLOSE;
// because close_internal_async() issues ReceiveReplayQueue.close_async() (which cannot
// be called from within a ReceiveReplayOperation), schedule the close rather than
// yield for it ... can't simply call the async .begin variant because, depending on
// the situation, it may not yield until it attempts to close the ReceiveReplayQueue,
// which is the problem we're attempting to work around
Idle.add(() => {
close_internal_async.begin(CloseReason.LOCAL_CLOSE, folder_reason, null);
return false;
});
}
//
// list_email_by_id variants
//
@ -1058,11 +1050,11 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
check_open("expunge_email_async");
check_ids("expunge_email_async", email_ids);
ExpungeEmail expunge = new ExpungeEmail(this, (Gee.List<ImapDB.EmailIdentifier>) email_ids,
RemoveEmail remove = new RemoveEmail(this, (Gee.List<ImapDB.EmailIdentifier>) email_ids,
cancellable);
replay_queue.schedule(expunge);
replay_queue.schedule(remove);
yield expunge.wait_for_ready_async(cancellable);
yield remove.wait_for_ready_async(cancellable);
}
private void check_open(string method) throws EngineError {

View file

@ -198,7 +198,7 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
// Adds everything in the expansion to the unfulfilled set with ImapDB's field requirements ...
// UIDs are returned if anything else needs to be added to them
protected async Gee.List<Imap.UID>? expand_vector_async(Imap.UID? initial_uid, int count) throws Error {
protected async Gee.Set<Imap.UID>? expand_vector_async(Imap.UID? initial_uid, int count) throws Error {
// watch out for situations where the entire folder is represented locally (i.e. no
// expansion necessary)
int remote_count = owner.get_remote_counts(null, null);
@ -209,7 +209,11 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
// is in process, in which case don't want to expand vector this moment because the
// vector is in flux
int local_count = yield owner.local_folder.get_email_count_async(
ImapDB.Folder.ListFlags.NONE, cancellable);
ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
// watch out for attempts to expand vector when it's expanded as far as it will go
if (local_count >= remote_count)
return null;
// determine low and high position for expansion ... default in most code paths for high
// is the SequenceNumber just below the lowest known message, unless no local messages
@ -278,28 +282,40 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
msg_set = new Imap.MessageSet.range_to_highest(low_pos);
}
debug("%s: Performing vector expansion using %s for initial_uid=%s count=%d actual_count=%d",
debug("%s: Performing vector expansion using %s for initial_uid=%s count=%d actual_count=%d local_count=%d remote_count=%d oldest_to_newest=%s",
owner.to_string(), msg_set.to_string(),
(initial_uid != null) ? initial_uid.to_string() : "(null)", count, actual_count);
(initial_uid != null) ? initial_uid.to_string() : "(null)", count, actual_count,
local_count, remote_count, flags.is_oldest_to_newest().to_string());
Gee.List<Geary.Email>? list = yield owner.remote_folder.list_email_async(msg_set,
Geary.Email.Field.NONE, cancellable);
Gee.List<Imap.UID> uids = new Gee.ArrayList<Imap.UID>();
Gee.Set<Imap.UID> uids = new Gee.HashSet<Imap.UID>();
if (list != null) {
// add all the new email to the unfulfilled list, which ensures (when replay_remote_async
// is called) that the fields are downloaded and added to the database
foreach (Geary.Email email in list) {
Imap.UID uid = ((ImapDB.EmailIdentifier) email.id).uid;
uids.add(uid);
add_unfulfilled_fields(uid, ImapDB.Folder.REQUIRED_FIELDS);
foreach (Geary.Email email in list)
uids.add(((ImapDB.EmailIdentifier) email.id).uid);
// remove any already stored locally
Gee.Collection<ImapDB.EmailIdentifier>? ids =
yield owner.local_folder.get_ids_async(uids, ImapDB.Folder.ListFlags.INCLUDE_MARKED_FOR_REMOVE,
cancellable);
if (ids != null && ids.size > 0) {
foreach (ImapDB.EmailIdentifier id in ids) {
assert(id.has_uid());
uids.remove(id.uid);
}
}
// for the remainder (not in local store), fetch the required fields
add_many_unfulfilled_fields(uids, ImapDB.Folder.REQUIRED_FIELDS);
}
debug("%s: Vector expansion completed (%d new email)", owner.to_string(),
(list != null) ? list.size : 0);
(uids != null) ? uids.size : 0);
return uids;
return uids != null && uids.size > 0 ? uids : null;
}
public override async void backout_local_async() throws Error {

View file

@ -136,7 +136,7 @@ private class Geary.ImapEngine.ListEmailByID : Geary.ImapEngine.AbstractListEmai
// If the vector is too short, expand it now
if (expansion_required) {
Gee.List<Imap.UID>? uids = yield expand_vector_async(initial_uid, count);
Gee.Set<Imap.UID>? uids = yield expand_vector_async(initial_uid, count);
if (uids != null) {
// add required_fields as well as basic required fields for new email
add_many_unfulfilled_fields(uids, required_fields);

View file

@ -4,16 +4,16 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
private class Geary.ImapEngine.ExpungeEmail : Geary.ImapEngine.SendReplayOperation {
private class Geary.ImapEngine.RemoveEmail : Geary.ImapEngine.SendReplayOperation {
private GenericFolder engine;
private Gee.List<ImapDB.EmailIdentifier> to_remove = new Gee.ArrayList<ImapDB.EmailIdentifier>();
private Cancellable? cancellable;
private Gee.Set<ImapDB.EmailIdentifier>? removed_ids = null;
private int original_count = 0;
public ExpungeEmail(GenericFolder engine, Gee.List<ImapDB.EmailIdentifier> to_remove,
public RemoveEmail(GenericFolder engine, Gee.List<ImapDB.EmailIdentifier> to_remove,
Cancellable? cancellable = null) {
base("ExpungeEmail");
base("RemoveEmail");
this.engine = engine;
@ -27,6 +27,7 @@ private class Geary.ImapEngine.ExpungeEmail : Geary.ImapEngine.SendReplayOperati
}
public override async ReplayOperation.Status replay_local_async() throws Error {
// if performing a full expunge, need to move on to replay_remote_async() for that
if (to_remove.size <= 0)
return ReplayOperation.Status.COMPLETED;
@ -64,9 +65,11 @@ private class Geary.ImapEngine.ExpungeEmail : Geary.ImapEngine.SendReplayOperati
}
public override async void backout_local_async() throws Error {
yield engine.local_folder.mark_removed_async(removed_ids, false, cancellable);
if (removed_ids != null && removed_ids.size > 0) {
yield engine.local_folder.mark_removed_async(removed_ids, false, cancellable);
engine.notify_email_inserted(removed_ids);
}
engine.notify_email_inserted(removed_ids);
engine.notify_email_count_changed(original_count, Geary.Folder.CountChangeReason.INSERTED);
}

View file

@ -22,7 +22,19 @@ private class Geary.ImapEngine.ReplayDisconnect : Geary.ImapEngine.ReceiveReplay
}
public override async ReplayOperation.Status replay_local_async() throws Error {
yield owner.do_replay_remote_disconnected(reason);
debug("%s ReplayDisconnect reason=%s", owner.to_string(), reason.to_string());
Geary.Folder.CloseReason remote_reason = reason.is_error()
? Geary.Folder.CloseReason.REMOTE_ERROR : Geary.Folder.CloseReason.REMOTE_CLOSE;
// because close_internal_async() may schedule a ReplayOperation before its first yield,
// that means a ReplayOperation is scheduling a ReplayOperation, which isn't something
// we want to encourage, so use the Idle queue to schedule close_internal_async
Idle.add(() => {
owner.close_internal_async.begin(Geary.Folder.CloseReason.LOCAL_CLOSE, remote_reason, null);
return false;
});
return ReplayOperation.Status.COMPLETED;
}

View file

@ -506,6 +506,13 @@ private class Geary.Imap.Folder : BaseObject {
StoreCommand store_cmd = new StoreCommand(msg_set, flags, true, false);
cmds.add(store_cmd);
// TODO: Only use old-school EXPUNGE when closing folder (or rely on CLOSE to do that work
// for us). See:
// http://redmine.yorba.org/issues/7532
//
// However, current client implementation doesn't properly close INBOX when application
// shuts down, which means deleted messages return at application start. See:
// http://redmine.yorba.org/issues/6865
if (msg_set.is_uid && session.capabilities.supports_uidplus())
cmds.add(new ExpungeCommand.uid(msg_set));
else
@ -564,6 +571,13 @@ private class Geary.Imap.Folder : BaseObject {
flags.add(MessageFlag.DELETED);
cmds.add(new StoreCommand(msg_set, flags, true, false));
// TODO: Only use old-school EXPUNGE when closing folder (or rely on CLOSE to do that work
// for us). See:
// http://redmine.yorba.org/issues/7532
//
// However, current client implementation doesn't properly close INBOX when application
// shuts down, which means deleted messages return at application start. See:
// http://redmine.yorba.org/issues/6865
if (msg_set.is_uid && session.capabilities.supports_uidplus())
cmds.add(new ExpungeCommand.uid(msg_set));
else

View file

@ -4,18 +4,30 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
/**
* A Semaphore is a broadcasting, manually-resetting {@link AbstractSempahore}.
*/
public class Geary.Nonblocking.Semaphore : Geary.Nonblocking.AbstractSemaphore {
public Semaphore(Cancellable? cancellable = null) {
base (true, false, cancellable);
}
}
/**
* An Event is a broadcasting, auto-resetting {@link AbstractSempahore}.
*/
public class Geary.Nonblocking.Event : Geary.Nonblocking.AbstractSemaphore {
public Event(Cancellable? cancellable = null) {
base (true, true, cancellable);
}
}
/**
* A Spinlock is a single-notifying, auto-resetting {@link AbstractSempahore}.
*/
public class Geary.Nonblocking.Spinlock : Geary.Nonblocking.AbstractSemaphore {
public Spinlock(Cancellable? cancellable = null) {
base (false, true, cancellable);