diff --git a/src/console/main.vala b/src/console/main.vala index b1a04542..0b0e78de 100644 --- a/src/console/main.vala +++ b/src/console/main.vala @@ -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, ""); + + 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, ""); + + 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, " "); diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index d0f6f277..85941990 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -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 { diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala b/src/engine/imap-engine/imap-engine-account-synchronizer.vala index 4230c67f..7d70b303 100644 --- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala +++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala @@ -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 + ); } } } diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index f90f3e05..26dbcf32 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -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 specials = - new Gee.HashMap(); - specials.set(special, minimal_folder); - promote_folders(specials); + Geary.Folder special_folder = this.folder_map.get(path); + promote_folders( + Collection.single_map( + 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(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 to_build = new Gee.ArrayList(); 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); } diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala b/src/engine/imap-engine/imap-engine-minimal-folder.vala index 53d5373b..80f55dad 100644 --- a/src/engine/imap-engine/imap-engine-minimal-folder.vala +++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala @@ -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; } diff --git a/src/engine/imap/command/imap-delete-command.vala b/src/engine/imap/command/imap-delete-command.vala new file mode 100644 index 00000000..e183f510 --- /dev/null +++ b/src/engine/imap/command/imap-delete-command.vala @@ -0,0 +1,26 @@ +/* + * Copyright 2018 Michael Gratton + * + * 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()); + } + +} diff --git a/src/engine/imap/message/imap-mailbox-specifier.vala b/src/engine/imap/message/imap-mailbox-specifier.vala index a41212bd..1984338b 100644 --- a/src/engine/imap/message/imap-mailbox-specifier.vala +++ b/src/engine/imap/message/imap-mailbox-specifier.vala @@ -127,6 +127,12 @@ public class Geary.Imap.MailboxSpecifier : BaseObject, Gee.Hashable 1 && parts[0] == "") { + parts.remove_at(0); + } + StringBuilder builder = new StringBuilder( is_inbox_name(parts[0]) ? inbox.name : parts[0]); diff --git a/src/engine/meson.build b/src/engine/meson.build index 3d99bcb8..222a0edc 100644 --- a/src/engine/meson.build +++ b/src/engine/meson.build @@ -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', diff --git a/src/engine/util/util-collection.vala b/src/engine/util/util-collection.vala index 092ed34d..8d750c0b 100644 --- a/src/engine/util/util-collection.vala +++ b/src/engine/util/util-collection.vala @@ -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 single(T element) { + Gee.Collection single = new Gee.LinkedList(); + single.add(element); + return single; +} + +/** Returns a modifiable map containing a single entry. */ +public Gee.Map single_map(K key, V value) { + Gee.Map single = new Gee.HashMap(); + single.set(key, value); + return single; +} + // A substitute for ArrayList.wrap() for compatibility with older versions of Gee. public Gee.ArrayList array_list_wrap(G[] a, owned Gee.EqualDataFunc? equal_func = null) { Gee.ArrayList list = new Gee.ArrayList((owned) equal_func); diff --git a/test/engine/imap/message/imap-mailbox-specifier-test.vala b/test/engine/imap/message/imap-mailbox-specifier-test.vala index 21964708..741f279e 100644 --- a/test/engine/imap/message/imap-mailbox-specifier-test.vala +++ b/test/engine/imap/message/imap-mailbox-specifier-test.vala @@ -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 + ); + } + }