Merge branch 'wip/795595-fix-special-folder-creation' into 'master'

Wip/795595 fix special folder creation

See merge request GNOME/geary!48
This commit is contained in:
Michael Gratton 2018-09-26 14:12:58 +00:00
commit c1bc3ee99d
10 changed files with 271 additions and 85 deletions

View file

@ -93,6 +93,8 @@ class ImapConsole : Gtk.Window {
"list",
"xlist",
"examine",
"create",
"delete",
"fetch",
"uid-fetch",
"fetch-fields",
@ -184,6 +186,14 @@ class ImapConsole : Gtk.Window {
examine(cmd, args);
break;
case "create":
create(cmd, args);
break;
case "delete":
@delete(cmd, args);
break;
case "fetch":
case "uid-fetch":
fetch(cmd, args);
@ -421,6 +431,28 @@ class ImapConsole : Gtk.Window {
this.cx.send_command(new Geary.Imap.ExamineCommand(new Geary.Imap.MailboxSpecifier(args[0])));
}
private void create(string cmd, string[] args) throws Error {
check_connected(cmd, args, 1, "<mailbox>");
status("Creating %s".printf(args[0]));
this.cx.send_command(
new Geary.Imap.CreateCommand(
new Geary.Imap.MailboxSpecifier(args[0])
)
);
}
private void @delete(string cmd, string[] args) throws Error {
check_connected(cmd, args, 1, "<mailbox>");
status("Deleting %s".printf(args[0]));
this.cx.send_command(
new Geary.Imap.DeleteCommand(
new Geary.Imap.MailboxSpecifier(args[0])
)
);
}
private void fetch(string cmd, string[] args) throws Error {
check_min_connected(cmd, args, 2, "<message-span> <data-item...>");

View file

@ -384,9 +384,10 @@ private class Geary.ImapDB.Account : BaseObject {
private void on_outbox_email_sent(Geary.RFC822.Message rfc822) {
email_sent(rfc822);
}
public async void clone_folder_async(Geary.Imap.Folder imap_folder, Cancellable? cancellable = null)
throws Error {
public async Folder clone_folder_async(Geary.Imap.Folder imap_folder,
GLib.Cancellable? cancellable = null)
throws GLib.Error {
check_open();
Geary.Imap.FolderProperties properties = imap_folder.properties;
@ -395,7 +396,9 @@ private class Geary.ImapDB.Account : BaseObject {
// XXX this should really be a db table constraint
Geary.ImapDB.Folder? folder = get_local_folder(path);
if (folder != null)
throw new EngineError.ALREADY_EXISTS(path.to_string());
throw new EngineError.ALREADY_EXISTS(
"Folder with path already exists: %s", path.to_string()
);
yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => {
// get the parent of this folder, creating parents if necessary ... ok if this fails,
@ -426,8 +429,11 @@ private class Geary.ImapDB.Account : BaseObject {
return Db.TransactionOutcome.COMMIT;
}, cancellable);
// XXX can't we create this from the INSERT above?
return yield fetch_folder_async(path, cancellable);
}
public async void delete_folder_async(Geary.Folder folder, Cancellable? cancellable)
throws Error {
check_open();
@ -444,11 +450,13 @@ private class Geary.ImapDB.Account : BaseObject {
debug("Can't delete folder %s because it has children", folder.to_string());
return Db.TransactionOutcome.ROLLBACK;
}
do_delete_folder(cx, folder_id, cancellable);
this.folder_refs.unset(path);
return Db.TransactionOutcome.COMMIT;
}, cancellable);
}
private void initialize_contacts(Cancellable? cancellable = null) throws Error {

View file

@ -100,40 +100,58 @@ private class Geary.ImapEngine.RefreshFolderSync : FolderOperation {
base(account, folder);
}
public override async void execute(Cancellable cancellable)
throws Error {
bool opened = false;
bool closed = false;
public override async void execute(GLib.Cancellable cancellable)
throws GLib.Error {
bool was_opened = false;
try {
yield this.folder.open_async(Folder.OpenFlags.NONE, cancellable);
opened = true;
yield this.folder.wait_for_remote_async(cancellable);
debug("Synchronising : %s", this.folder.to_string());
was_opened = true;
debug("Synchronising %s", this.folder.to_string());
yield sync_folder(cancellable);
} finally {
if (opened) {
try {
// don't pass in the Cancellable; really need this
// to complete in all cases
closed = yield this.folder.close_async();
if (closed) {
// If the folder was actually closing, wait
// for it here to completely close so that its
// session has a chance to exit IMAP Selected
// state when released, allowing the next sync
// op to reuse the same session. Here we
// definitely want to use the cancellable so
// the wait can be interrupted.
yield this.folder.wait_for_close_async(cancellable);
}
} catch (Error err) {
debug(
"%s: Error closing folder %s: %s",
this.account.to_string(),
this.folder.to_string(),
err.message
);
} catch (GLib.IOError.CANCELLED err) {
// All good
} catch (EngineError.ALREADY_CLOSED err) {
// Failed to open the folder, which could be because the
// network went away, or because the remote folder went
// away. Either way don't bother reporting it.
debug(
"Folder failed to open %s: %s",
this.folder.to_string(),
err.message
);
} catch (GLib.Error err) {
this.account.report_problem(
new ServiceProblemReport(
ProblemType.GENERIC_ERROR,
this.account.information,
this.account.information.imap,
err
)
);
}
if (was_opened) {
try {
// don't pass in the Cancellable; really need this
// to complete in all cases
if (yield this.folder.close_async(null)) {
// If the folder was actually closing, wait
// for it here to completely close so that its
// session has a chance to exit IMAP Selected
// state when released, allowing the next sync
// op to reuse the same session. Here we
// definitely want to use the cancellable so
// the wait can be interrupted.
yield this.folder.wait_for_close_async(cancellable);
}
} catch (Error err) {
debug(
"%s: Error closing folder %s: %s",
this.account.to_string(),
this.folder.to_string(),
err.message
);
}
}
}

View file

@ -442,7 +442,9 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
folder = this.folder_map.get(path);
if (folder == null) {
throw new EngineError.NOT_FOUND(path.to_string());
throw new EngineError.NOT_FOUND(
"Folder not found: %s", path.to_string()
);
}
}
return folder;
@ -668,7 +670,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
Geary.SpecialFolderType special,
Cancellable? cancellable)
throws Error {
MinimalFolder? minimal_folder = null;
Geary.FolderPath? path = information.get_special_folder_path(special);
if (path != null) {
debug("Previously used %s for special folder %s", path.to_string(), special.to_string());
@ -696,22 +697,49 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
information.set_special_folder_path(special, path);
}
if (path in folder_map.keys) {
debug("Promoting %s to special folder %s", path.to_string(), special.to_string());
minimal_folder = folder_map.get(path);
} else {
debug("Creating %s to use as special folder %s", path.to_string(), special.to_string());
// TODO: ignore error due to already existing.
yield remote.create_folder_async(path, special, cancellable);
minimal_folder = (MinimalFolder) yield fetch_folder_async(path, cancellable);
if (!this.folder_map.has_key(path)) {
debug("Creating \"%s\" to use as special folder %s",
path.to_string(), special.to_string());
GLib.Error? created_err = null;
try {
yield remote.create_folder_async(path, special, cancellable);
} catch (GLib.Error err) {
// Hang on to the error since the folder might exist
// on the remote, so try fetching it anyway.
created_err = err;
}
Imap.Folder? remote_folder = null;
try {
remote_folder = yield remote.fetch_folder_async(
path, cancellable
);
} catch (GLib.Error err) {
// If we couldn't fetch it after also failing to
// create it, it's probably due to the problem
// creating it, so throw that error instead.
if (created_err != null) {
throw created_err;
} else {
throw err;
}
}
ImapDB.Folder local_folder = yield this.local.clone_folder_async(
remote_folder, cancellable
);
add_folders(Collection.single(local_folder), created_err != null);
}
Gee.Map<Geary.SpecialFolderType,Geary.Folder> specials =
new Gee.HashMap<Geary.SpecialFolderType,Geary.Folder>();
specials.set(special, minimal_folder);
promote_folders(specials);
Geary.Folder special_folder = this.folder_map.get(path);
promote_folders(
Collection.single_map<Geary.SpecialFolderType,Geary.Folder>(
special, special_folder
)
);
return minimal_folder;
return special_folder;
}
/**
@ -1220,10 +1248,11 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
remote_folder.path.to_string(), update_error.message);
}
// set the engine folder's special type
// (but only promote, not demote, since getting the special folder type via its
// properties relies on the optional XLIST extension)
// use this iteration to add discovered properties to map
// set the engine folder's special type (but only promote,
// not demote, since getting the special folder type via
// its properties relies on the optional SPECIAL-USE or
// XLIST extensions) use this iteration to add discovered
// properties to map
if (minimal_folder.special_folder_type == SpecialFolderType.NONE)
minimal_folder.set_special_folder_type(remote_folder.properties.attrs.get_special_folder_type());
}
@ -1240,27 +1269,19 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
.map<Geary.Folder>(e => (Geary.Folder) e.value)
.to_array_list();
// For folders to add, clone them and their properties locally
// For folders to add, clone them and their properties
// locally, then add to the account
ImapDB.Account local = ((GenericAccount) this.account).local;
foreach (Geary.Imap.Folder remote_folder in to_add) {
try {
yield local.clone_folder_async(remote_folder, cancellable);
} catch (Error err) {
debug("Unable to add/remove folder %s to local store: %s", remote_folder.path.to_string(),
err.message);
}
}
// Create Geary.Folder objects for all added folders
Gee.ArrayList<ImapDB.Folder> to_build = new Gee.ArrayList<ImapDB.Folder>();
foreach (Geary.Imap.Folder remote_folder in to_add) {
try {
to_build.add(yield local.fetch_folder_async(remote_folder.path, cancellable));
} catch (Error convert_err) {
// This isn't fatal, but irksome ... in the future, when local folders are
// removed, it's possible for one to disappear between cloning it and fetching
// it
debug("Unable to fetch local folder after cloning: %s", convert_err.message);
to_build.add(
yield local.clone_folder_async(remote_folder, cancellable)
);
} catch (Error err) {
debug("Unable to clone folder %s in local store: %s",
remote_folder.path.to_string(),
err.message);
}
}
this.generic_account.add_folders(to_build, false);
@ -1304,11 +1325,9 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
// Ensure each of the important special folders we need already exist
foreach (Geary.SpecialFolderType special in this.specials) {
try {
if (this.generic_account.get_special_folder(special) == null) {
yield this.generic_account.ensure_special_folder_async(
remote, special, cancellable
);
}
yield this.generic_account.ensure_special_folder_async(
remote, special, cancellable
);
} catch (Error e) {
warning("Unable to ensure special folder %s: %s", special.to_string(), e.message);
}

View file

@ -954,15 +954,24 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Imap.FolderSession? session = null;
try {
session = yield this._account.claim_folder_session(this.path, cancellable);
session = yield this._account.claim_folder_session(
this.path, cancellable
);
} catch (IOError.CANCELLED err) {
// Fine, just bail out
return;
} catch (EngineError.NOT_FOUND err) {
// Folder no longer exists, so force closed
yield force_close(
CloseReason.LOCAL_CLOSE, CloseReason.REMOTE_ERROR
);
return;
} catch (Error err) {
if (!(err is IOError.CANCELLED)) {
// Notify that there was a connection error, but don't
// force the folder closed, since it might come good again
// if the user fixes an auth problem or the network comes
// back or whatever.
notify_open_failed(Folder.OpenFailed.REMOTE_ERROR, err);
}
// Notify that there was a connection error, but don't
// force the folder closed, since it might come good again
// if the user fixes an auth problem or the network comes
// back or whatever.
notify_open_failed(Folder.OpenFailed.REMOTE_ERROR, err);
return;
}

View file

@ -0,0 +1,26 @@
/*
* Copyright 2018 Michael Gratton <mike@vee.net>
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
/**
* The RFC 3501 DELETE command.
*
* Deletes the given mailbox. Per the RFC, this must not be used to
* delete mailboxes with child (inferior) mailboxes and that also are
* marked \Noselect.
*
* See [[http://tools.ietf.org/html/rfc3501#section-6.3.4]]
*/
public class Geary.Imap.DeleteCommand : Command {
public const string NAME = "DELETE";
public DeleteCommand(MailboxSpecifier mailbox) {
base(NAME);
this.args.add(mailbox.to_parameter());
}
}

View file

@ -127,6 +127,12 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable<MailboxSpeci
throw new ImapError.INVALID("Path has more than one part but no delimiter given");
}
// Don't include the root if it is an empty string so that
// mailboxes do not begin with the delim.
if (parts.size > 1 && parts[0] == "") {
parts.remove_at(0);
}
StringBuilder builder = new StringBuilder(
is_inbox_name(parts[0]) ? inbox.name : parts[0]);

View file

@ -99,6 +99,7 @@ geary_engine_vala_sources = files(
'imap/command/imap-compress-command.vala',
'imap/command/imap-copy-command.vala',
'imap/command/imap-create-command.vala',
'imap/command/imap-delete-command.vala',
'imap/command/imap-examine-command.vala',
'imap/command/imap-expunge-command.vala',
'imap/command/imap-fetch-command.vala',

View file

@ -12,6 +12,20 @@ public inline bool is_empty(Gee.Collection? c) {
return c == null || c.size == 0;
}
/** Returns a modifiable collection containing a single element. */
public Gee.Collection<T> single<T>(T element) {
Gee.Collection<T> single = new Gee.LinkedList<T>();
single.add(element);
return single;
}
/** Returns a modifiable map containing a single entry. */
public Gee.Map<K,V> single_map<K,V>(K key, V value) {
Gee.Map<K,V> single = new Gee.HashMap<K,V>();
single.set(key, value);
return single;
}
// A substitute for ArrayList<G>.wrap() for compatibility with older versions of Gee.
public Gee.ArrayList<G> array_list_wrap<G>(G[] a, owned Gee.EqualDataFunc<G>? equal_func = null) {
Gee.ArrayList<G> list = new Gee.ArrayList<G>((owned) equal_func);

View file

@ -12,6 +12,7 @@ class Geary.Imap.MailboxSpecifierTest : TestCase {
base("Geary.Imap.MailboxSpecifierTest");
add_test("to_parameter", to_parameter);
add_test("from_parameter", from_parameter);
add_test("from_folder_path", from_folder_path);
}
public void to_parameter() throws Error {
@ -57,4 +58,56 @@ class Geary.Imap.MailboxSpecifierTest : TestCase {
);
}
public void from_folder_path() throws Error {
MockFolderRoot empty_root = new MockFolderRoot("");
MailboxSpecifier empty_inbox = new MailboxSpecifier("Inbox");
assert_string(
"Foo",
new MailboxSpecifier.from_folder_path(
empty_root.get_child("Foo"), empty_inbox, "$"
).name
);
assert_string(
"Foo$Bar",
new MailboxSpecifier.from_folder_path(
empty_root.get_child("Foo").get_child("Bar"), empty_inbox, "$"
).name
);
assert_string(
"Inbox",
new MailboxSpecifier.from_folder_path(
empty_root.get_child(MailboxSpecifier.CANONICAL_INBOX_NAME),
empty_inbox,
"$"
).name
);
MockFolderRoot non_empty_root = new MockFolderRoot("Root");
MailboxSpecifier non_empty_inbox = new MailboxSpecifier("Inbox");
assert_string(
"Root$Foo",
new MailboxSpecifier.from_folder_path(
non_empty_root.get_child("Foo"),
non_empty_inbox,
"$"
).name
);
assert_string(
"Root$Foo$Bar",
new MailboxSpecifier.from_folder_path(
non_empty_root.get_child("Foo").get_child("Bar"),
non_empty_inbox,
"$"
).name
);
assert_string(
"Root$INBOX",
new MailboxSpecifier.from_folder_path(
non_empty_root.get_child(MailboxSpecifier.CANONICAL_INBOX_NAME),
non_empty_inbox,
"$"
).name
);
}
}