From 7449e10b3eb12be40f2123a0a4414b1e123fe12e Mon Sep 17 00:00:00 2001
From: Michael Gratton <mike@vee.net>
Date: Wed, 26 Sep 2018 23:28:43 +1000
Subject: [PATCH] Correctly handle creating special folders when they do not
 exist

---
 .../imap-engine-generic-account.vala          | 71 +++++++++++++------
 src/engine/util/util-collection.vala          | 14 ++++
 2 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala
index 3edae386..26dbcf32 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -670,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());
@@ -698,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;
     }
 
     /**
@@ -1222,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());
         }
@@ -1298,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/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<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);