[geary/wip/721828-undo] First stab
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/721828-undo] First stab
- Date: Tue, 23 Dec 2014 02:54:00 +0000 (UTC)
commit 848996db78ba9ac7fd6fc6a7c152468e6b6db9a9
Author: Jim Nelson <jim yorba org>
Date: Mon Dec 22 18:51:08 2014 -0800
First stab
Plan is to rely on COPYUID response to know UIDs of messages in
destination folder. Gmail will no longer delete/expunge messages to
archive, but will move them to All Mail (just like moving to Trash)
and use COPYUID. This unifies all approaches and allows for a simple
undo command.
Questions remain if the undo stack is maintained by the client or if
the engine supplies a simple, one-operation "stack", which doesn't
merely simplify the implementation on the client side but also allows
for the Account object to directly manage UIDs; otherwise, the COPYUID
UIDs must be recorded in the database in order to generate valid
ImapDB.EmailIdentifiers.
src/client/application/geary-controller.vala | 10 +-
.../gmail/imap-engine-gmail-folder.vala | 3 +
.../replay-ops/imap-engine-copy-email.vala | 15 ++-
.../replay-ops/imap-engine-move-email.vala | 13 ++-
src/engine/imap/api/imap-folder.vala | 24 ++++-
src/engine/imap/command/imap-message-set.vala | 117 ++++++++++++++++++++
src/engine/imap/parameter/imap-list-parameter.vala | 38 +++++++
.../imap/parameter/imap-number-parameter.vala | 8 +-
.../imap/parameter/imap-string-parameter.vala | 18 +++
.../imap/response/imap-response-code-type.vala | 1 +
src/engine/imap/response/imap-response-code.vala | 20 ++++
src/engine/imap/transport/imap-deserializer.vala | 2 +
12 files changed, 258 insertions(+), 11 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 39f5c75..0116df7 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -2265,10 +2265,12 @@ public class GearyController : Geary.BaseObject {
public bool confirm_delete(int num_messages) {
main_window.present();
- AlertDialog dialog = new ConfirmationDialog(main_window, ngettext(
- "Do you want to permanently delete this message?",
- "Do you want to permanently delete these messages?", num_messages),
- null, _("Delete"));
+ AlertDialog dialog = new ConfirmationDialog(main_window,
+ ngettext(
+ "Do you want to permanently delete this message? This operation cannot be undone.",
+ "Do you want to permanently delete these messages? This operation cannot be undone.",
+ num_messages
+ ), null, _("Delete"));
return (dialog.run() == Gtk.ResponseType.OK);
}
diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
index e8839c0..0d233ec 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
@@ -19,6 +19,9 @@ private class Geary.ImapEngine.GmailFolder : MinimalFolder, FolderSupport.Archiv
public async void archive_email_async(Gee.List<Geary.EmailIdentifier> email_ids,
Cancellable? cancellable = null) throws Error {
+ // TODO: Use move_email_async("All Mail") here; Gmail will do the right thing and report
+ // it was copied with the All Mail UID (in other words, no actual copy is performed).
+ // Stash returned All Mail identifier as the Undo identifier
yield expunge_email_async(email_ids, cancellable);
}
}
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala
b/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala
index 0d68888..42ccc09 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala
@@ -5,6 +5,8 @@
*/
private class Geary.ImapEngine.CopyEmail : Geary.ImapEngine.SendReplayOperation {
+ public Gee.Set<Imap.UID>? destination_uids { get; private set; default = null; }
+
private MinimalFolder engine;
private Gee.HashSet<ImapDB.EmailIdentifier> to_copy = new Gee.HashSet<ImapDB.EmailIdentifier>();
private Geary.FolderPath destination;
@@ -45,9 +47,18 @@ private class Geary.ImapEngine.CopyEmail : Geary.ImapEngine.SendReplayOperation
ImapDB.Folder.ListFlags.NONE, cancellable);
if (uids != null && uids.size > 0) {
+ Gee.Set<Imap.UID> acc_uids = new Gee.HashSet<Imap.UID>();
+
Gee.List<Imap.MessageSet> msg_sets = Imap.MessageSet.uid_sparse(uids);
- foreach (Imap.MessageSet msg_set in msg_sets)
- yield engine.remote_folder.copy_email_async(msg_set, destination, cancellable);
+ foreach (Imap.MessageSet msg_set in msg_sets) {
+ Gee.Set<Imap.UID>? dest_uids = yield engine.remote_folder.copy_email_async(msg_set,
+ destination, cancellable);
+ if (dest_uids != null)
+ acc_uids.add_all(dest_uids);
+ }
+
+ if (acc_uids.size > 0)
+ destination_uids = acc_uids;
}
return ReplayOperation.Status.COMPLETED;
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 236932b..a93a742 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
@@ -5,6 +5,8 @@
*/
private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation {
+ public Gee.Set<Imap.UID>? destination_uids { get; private set; default = null; }
+
private MinimalFolder engine;
private Gee.List<ImapDB.EmailIdentifier> to_move = new Gee.ArrayList<ImapDB.EmailIdentifier>();
private Geary.FolderPath destination;
@@ -67,13 +69,22 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation
if (cancellable != null && cancellable.is_cancelled())
throw new IOError.CANCELLED("Move email to %s cancelled", engine.remote_folder.to_string());
+ Gee.Set<Imap.UID> acc_uids = new Gee.HashSet<Imap.UID>();
+
Gee.List<Imap.MessageSet> msg_sets = Imap.MessageSet.uid_sparse(
ImapDB.EmailIdentifier.to_uids(moved_ids));
foreach (Imap.MessageSet msg_set in msg_sets) {
- yield engine.remote_folder.copy_email_async(msg_set, destination, null);
+ Gee.Set<Imap.UID>? dest_uids = yield engine.remote_folder.copy_email_async(msg_set,
+ destination, null);
+ if (dest_uids != null)
+ acc_uids.add_all(dest_uids);
+
yield engine.remote_folder.remove_email_async(msg_set, null);
}
+ if (acc_uids.size > 0)
+ destination_uids = acc_uids;
+
return ReplayOperation.Status.COMPLETED;
}
diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala
index 82f316a..f40ae46 100644
--- a/src/engine/imap/api/imap-folder.vala
+++ b/src/engine/imap/api/imap-folder.vala
@@ -675,15 +675,33 @@ private class Geary.Imap.Folder : BaseObject {
yield exec_commands_async(cmds, null, null, cancellable);
}
- public async void copy_email_async(MessageSet msg_set, Geary.FolderPath destination,
+ public async Gee.Set<UID>? copy_email_async(MessageSet msg_set, Geary.FolderPath destination,
Cancellable? cancellable) throws Error {
check_open();
CopyCommand cmd = new CopyCommand(msg_set,
new MailboxSpecifier.from_folder_path(destination, null));
- yield exec_commands_async(Geary.iterate<Command>(cmd).to_array_list(), null,
- null, cancellable);
+ Gee.Map<Command, StatusResponse> responses = yield exec_commands_async(
+ Geary.iterate<Command>(cmd).to_array_list(), null, null, cancellable);
+
+ if (!responses.has_key(cmd))
+ return null;
+
+ StatusResponse response = responses.get(cmd);
+ if (response.response_code != null) {
+ Gee.List<UID>? destination_uids = null;
+ try {
+ response.response_code.get_copyuid(null, null, out destination_uids);
+ } catch (ImapError ierr) {
+ debug("Unable to retrieve COPYUID UIDs: %s", ierr.message);
+ }
+
+ if (destination_uids != null && destination_uids.size > 0)
+ return Geary.traverse<UID>(destination_uids).to_hash_set();
+ }
+
+ return null;
}
public async Gee.SortedSet<Imap.UID>? search_async(SearchCriteria criteria, Cancellable? cancellable)
diff --git a/src/engine/imap/command/imap-message-set.vala b/src/engine/imap/command/imap-message-set.vala
index 85af92a..39459b3 100644
--- a/src/engine/imap/command/imap-message-set.vala
+++ b/src/engine/imap/command/imap-message-set.vala
@@ -20,6 +20,8 @@ public class Geary.Imap.MessageSet : BaseObject {
// etc.)
private const int MAX_SPARSE_VALUES_PER_SET = 50;
+ private delegate void ParserCallback(int64 value) throws ImapError;
+
/**
* True if the { link MessageSet} was created with a UID or a UID range.
*
@@ -109,6 +111,121 @@ public class Geary.Imap.MessageSet : BaseObject {
}
/**
+ * Parses a string representing a { link MessageSet} into a List of { link SequenceNumber}s.
+ *
+ * See the note at { link parse_uid} about limitations of this method.
+ *
+ * Returns null if the string or parsed set is empty.
+ *
+ * @see uid_parse
+ */
+ public static Gee.List<SequenceNumber>? parse(string str) throws ImapError {
+ Gee.List<SequenceNumber> seq_nums = new Gee.ArrayList<SequenceNumber>();
+ parse_string(str, (value) => { seq_nums.add(new SequenceNumber.checked(value)); });
+
+ return seq_nums.size > 0 ? seq_nums : null;
+ }
+
+ /**
+ * Parses a string representing a { link MessageSet} into a List of { link UID}s.
+ *
+ * Note that this is currently designed for parsing message set responses from the server,
+ * specifically for COPYUID, which has some limitations in what may be returned. Notably, the
+ * asterisk ("*") symbol may not be returned. Thus, this method does not properly parse
+ * the full range of message set notation and can't even be trusted to reverse-parse the output
+ * of this class. A full implementation might be considered later.
+ *
+ * Because COPYUID returns values in the order copied, this method returns a List, not a Set,
+ * of values. They are in the order received (and properly deal with ranges in backwards
+ * order, i.e. "12:10"). This means duplicates may be encountered multiple times if the server
+ * returns those values.
+ *
+ * Returns null if the string or parsed set is empty.
+ */
+ public static Gee.List<UID>? uid_parse(string str) throws ImapError {
+ Gee.List<UID> uids = new Gee.ArrayList<UID>();
+ parse_string(str, (value) => { uids.add(new UID.checked(value)); });
+
+ return uids.size > 0 ? uids : null;
+ }
+
+ private static void parse_string(string str, ParserCallback cb) throws ImapError {
+ StringBuilder acc = new StringBuilder();
+ int64 start_range = -1;
+ bool in_range = false;
+
+ unichar ch;
+ int index = 0;
+ while (str.get_next_char(ref index, out ch)) {
+ // if number, add to accumulator
+ if (ch.isdigit()) {
+ acc.append_unichar(ch);
+
+ continue;
+ }
+
+ // look for special characters and deal with them
+ switch (ch) {
+ case ':':
+ // range separator
+ if (in_range)
+ throw new ImapError.INVALID("Bad range specifier in message set \"%s\"", str);
+
+ in_range = true;
+
+ // store current accumulated value as start of range
+ start_range = int64.parse(acc.str);
+ acc = new StringBuilder();
+ break;
+
+ case ',':
+ // number separator
+
+ // if in range, treat as end-of-range
+ if (in_range) {
+ // don't be forgiving here
+ if (String.is_empty(acc.str))
+ throw new ImapError.INVALID("Bad range specifier in message set \"%s\"", str);
+
+ process_range(start_range, int64.parse(acc.str), cb);
+ in_range = false;
+ } else {
+ // Be forgiving here
+ if (String.is_empty(acc.str))
+ continue;
+
+ cb(int64.parse(acc.str));
+ }
+
+ // reset accumulator
+ acc = new StringBuilder();
+ break;
+
+ default:
+ // unknown character, treat with great violence
+ throw new ImapError.INVALID("Bad character '%s' in message set \"%s\"",
+ ch.to_string(), str);
+ }
+ }
+
+ // report last bit remaining in accumulator
+ if (!String.is_empty(acc.str)) {
+ if (in_range)
+ process_range(start_range, int64.parse(acc.str), cb);
+ else
+ cb(int64.parse(acc.str));
+ } else if (in_range) {
+ throw new ImapError.INVALID("Incomplete range specifier in message set \"%s\"", str);
+ }
+ }
+
+ private static void process_range(int64 start, int64 end, ParserCallback cb) throws ImapError {
+ int64 count_by = (start <= end) ? 1 : -1;
+ for (int64 ctr = start; ctr != end + count_by; ctr += count_by)
+ cb(ctr);
+ }
+
+ /**
* Convert a collection of { link SequenceNumber}s into a list of { link MessageSet}s.
*
* Although this could return a single MessageSet, large collections could create an IMAP
diff --git a/src/engine/imap/parameter/imap-list-parameter.vala
b/src/engine/imap/parameter/imap-list-parameter.vala
index e561102..4e57d95 100644
--- a/src/engine/imap/parameter/imap-list-parameter.vala
+++ b/src/engine/imap/parameter/imap-list-parameter.vala
@@ -325,6 +325,44 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
}
//
+ // Number retrieval
+ //
+
+ /**
+ * Returns a { link NumberParameter} at index, null if not of that type.
+ *
+ * @see get_if
+ */
+ public NumberParameter? get_if_number(int index) {
+ return (NumberParameter?) get_if(index, typeof(NumberParameter));
+ }
+
+ /**
+ * Returns a { link NumberParameter} at index.
+ *
+ * Like { link get_as_string}, this method will attempt some coercion. In this case,
+ * { link QuotedStringParameter} and { link UnquotedStringParameter}s will be converted to
+ * NumberParameter, if appropriate.
+ */
+ public NumberParameter get_as_number(int index) throws ImapError {
+ Parameter param = get_required(index);
+
+ NumberParameter? numberp = param as NumberParameter;
+ if (numberp != null)
+ return numberp;
+
+ StringParameter? stringp = param as StringParameter;
+ if (stringp != null) {
+ numberp = stringp.coerce_to_number_parameter();
+ if (numberp != null)
+ return numberp;
+ }
+
+ throw new ImapError.TYPE_ERROR("Parameter %d not of type number or string (is %s)", index,
+ param.get_type().name());
+ }
+
+ //
// List retrieval
//
diff --git a/src/engine/imap/parameter/imap-number-parameter.vala
b/src/engine/imap/parameter/imap-number-parameter.vala
index e32c3cc..260db0d 100644
--- a/src/engine/imap/parameter/imap-number-parameter.vala
+++ b/src/engine/imap/parameter/imap-number-parameter.vala
@@ -5,7 +5,8 @@
*/
/**
- * A representation of a numerical { link Parameter} in an IMAP { link Command}.
+ * A representation of a numerical { link Parameter} in an IMAP { link Command} or
+ * { link ServerResponse}.
*
* See [[http://tools.ietf.org/html/rfc3501#section-4.2]]
*/
@@ -86,6 +87,11 @@ public class Geary.Imap.NumberParameter : UnquotedStringParameter {
has_nonzero = true;
}
+ // watch for negative but no numeric portion
+ if (is_negative && str.length == 1)
+ return false;
+
+ // no such thing as negative zero
if (is_negative && !has_nonzero)
is_negative = false;
diff --git a/src/engine/imap/parameter/imap-string-parameter.vala
b/src/engine/imap/parameter/imap-string-parameter.vala
index 01b6f0b..a1978cf 100644
--- a/src/engine/imap/parameter/imap-string-parameter.vala
+++ b/src/engine/imap/parameter/imap-string-parameter.vala
@@ -188,5 +188,23 @@ public abstract class Geary.Imap.StringParameter : Geary.Imap.Parameter {
return int64.parse(ascii).clamp(clamp_min, clamp_max);
}
+
+ /**
+ * Attempts to coerce a { link StringParameter} into a { link NumberParameter}.
+ *
+ * Returns null if unsuitable for a NumberParameter.
+ *
+ * @see NumberParameter.is_ascii_number
+ */
+ public NumberParameter? coerce_to_number_parameter() {
+ NumberParameter? numberp = this as NumberParameter;
+ if (numberp != null)
+ return numberp;
+
+ if (NumberParameter.is_ascii_numeric(ascii, null))
+ return new NumberParameter.from_ascii(ascii);
+
+ return null;
+ }
}
diff --git a/src/engine/imap/response/imap-response-code-type.vala
b/src/engine/imap/response/imap-response-code-type.vala
index 3bb0c84..c5c52be 100644
--- a/src/engine/imap/response/imap-response-code-type.vala
+++ b/src/engine/imap/response/imap-response-code-type.vala
@@ -23,6 +23,7 @@ public class Geary.Imap.ResponseCodeType : BaseObject, Gee.Hashable<ResponseCode
public const string BADCHARSET = "badcharset";
public const string CAPABILITY = "capability";
public const string CLIENTBUG = "clientbug";
+ public const string COPYUID = "copyuid";
public const string MYRIGHTS = "myrights";
public const string NEWNAME = "newname";
public const string NONEXISTANT = "nonexistant";
diff --git a/src/engine/imap/response/imap-response-code.vala
b/src/engine/imap/response/imap-response-code.vala
index fbd8ce8..e8887e2 100644
--- a/src/engine/imap/response/imap-response-code.vala
+++ b/src/engine/imap/response/imap-response-code.vala
@@ -89,6 +89,26 @@ public class Geary.Imap.ResponseCode : Geary.Imap.ListParameter {
return capabilities;
}
+ /**
+ * Parses the { link ResponseCode} into UIDPLUS' COPYUID response, if possible.
+ *
+ * Note that the { link UID}s are returned from the server in the order the messages
+ * were copied.
+ *
+ * See [[http://tools.ietf.org/html/rfc4315#section-3]]
+ *
+ * @throws ImapError.INVALID if not COPYUID.
+ */
+ public void get_copyuid(out UIDValidity uidvalidity, out Gee.List<UID>? source_uids,
+ out Gee.List<UID>? destination_uids) throws ImapError {
+ if (!get_response_code_type().is_value(ResponseCodeType.COPYUID))
+ throw new ImapError.INVALID("Not COPYUID response code: %s", to_string());
+
+ uidvalidity = new UIDValidity.checked(get_as_number(1).as_int64());
+ source_uids = MessageSet.uid_parse(get_as_string(2).ascii);
+ destination_uids = MessageSet.uid_parse(get_as_string(3).ascii);
+ }
+
public override string to_string() {
return "[%s]".printf(stringize_list());
}
diff --git a/src/engine/imap/transport/imap-deserializer.vala
b/src/engine/imap/transport/imap-deserializer.vala
index 63cfd2f..3f6be64 100644
--- a/src/engine/imap/transport/imap-deserializer.vala
+++ b/src/engine/imap/transport/imap-deserializer.vala
@@ -449,6 +449,8 @@ public class Geary.Imap.Deserializer : BaseObject {
if (quoted)
save_parameter(new QuotedStringParameter(str));
+ else if (NumberParameter.is_ascii_numeric(str, null))
+ save_parameter(new NumberParameter.from_ascii(str));
else
save_parameter(new UnquotedStringParameter(str));
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
