[geary/mjog/email-templates: 1/6] Geary.AccountInformation: Rework how special use folder paths are stored



commit 400850cc44dcac53ab2218fbd023a8f1ceb34eb5
Author: Michael Gratton <mike vee net>
Date:   Wed Apr 22 18:56:32 2020 +1000

    Geary.AccountInformation: Rework how special use folder paths are stored
    
    Use a simple list of strings to store path steps, since we don't know
    what the root will be in advance, and this leads to errors if callers
    try to compare the info's paths to (say) an actual IMAP path.
    
    Store path steps in a SpecialUse to path step map property, rather than
    as individual properties for each special use, to reduce repetition.

 src/client/accounts/accounts-manager.vala          | 126 ++++++++-------
 src/engine/api/geary-account-information.vala      | 169 +++++++++------------
 .../imap-engine/imap-engine-generic-account.vala   |  26 ++--
 test/client/accounts/accounts-manager-test.vala    |  21 +--
 4 files changed, 162 insertions(+), 180 deletions(-)
---
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index ab2e390e..f6d5749f 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -1108,17 +1108,29 @@ public class Accounts.AccountConfigV1 : AccountConfig, GLib.Object {
 
         Geary.ConfigFile.Group folder_config =
             config.get_group(GROUP_FOLDERS);
-        account.archive_folder_path = load_folder(folder_config, FOLDER_ARCHIVE);
-        account.drafts_folder_path = load_folder(folder_config, FOLDER_DRAFTS);
-        account.sent_folder_path = load_folder(folder_config, FOLDER_SENT);
+        account.set_folder_steps_for_use(
+            ARCHIVE, folder_config.get_string_list(FOLDER_ARCHIVE)
+        );
+        account.set_folder_steps_for_use(
+            DRAFTS, folder_config.get_string_list(FOLDER_DRAFTS)
+        );
+        account.set_folder_steps_for_use(
+            SENT, folder_config.get_string_list(FOLDER_SENT)
+        );
         // v3.32-3.36 used spam instead of junk
         if (folder_config.has_key(FOLDER_SPAM)) {
-            account.junk_folder_path = load_folder(folder_config, FOLDER_SPAM);
+            account.set_folder_steps_for_use(
+                JUNK, folder_config.get_string_list(FOLDER_SPAM)
+            );
         }
         if (folder_config.has_key(FOLDER_JUNK)) {
-            account.junk_folder_path = load_folder(folder_config, FOLDER_JUNK);
+            account.set_folder_steps_for_use(
+                JUNK, folder_config.get_string_list(FOLDER_JUNK)
+            );
         }
-        account.trash_folder_path = load_folder(folder_config, FOLDER_TRASH);
+        account.set_folder_steps_for_use(
+            TRASH, folder_config.get_string_list(FOLDER_TRASH)
+        );
 
         return account;
     }
@@ -1150,31 +1162,39 @@ public class Accounts.AccountConfigV1 : AccountConfig, GLib.Object {
 
         Geary.ConfigFile.Group folder_config =
             config.get_group(GROUP_FOLDERS);
-        save_folder(folder_config, FOLDER_ARCHIVE, account.archive_folder_path);
-        save_folder(folder_config, FOLDER_DRAFTS, account.drafts_folder_path);
-        save_folder(folder_config, FOLDER_SENT, account.sent_folder_path);
-        save_folder(folder_config, FOLDER_JUNK, account.junk_folder_path);
-        save_folder(folder_config, FOLDER_TRASH, account.trash_folder_path);
-    }
-
-    private void save_folder(Geary.ConfigFile.Group config,
-                             string key,
-                             Geary.FolderPath? path) {
-        if (path != null) {
-            config.set_string_list(
-                key, new Gee.ArrayList<string>.wrap(path.as_array())
-            );
-        }
+        save_folder(
+            folder_config,
+            FOLDER_ARCHIVE,
+            account.get_folder_steps_for_use(ARCHIVE)
+        );
+        save_folder(
+            folder_config,
+            FOLDER_DRAFTS,
+            account.get_folder_steps_for_use(DRAFTS)
+        );
+        save_folder(
+            folder_config,
+            FOLDER_SENT,
+            account.get_folder_steps_for_use(SENT)
+        );
+        save_folder(
+            folder_config,
+            FOLDER_JUNK,
+            account.get_folder_steps_for_use(JUNK)
+        );
+        save_folder(
+            folder_config,
+            FOLDER_TRASH,
+            account.get_folder_steps_for_use(TRASH)
+        );
     }
 
-    private Geary.FolderPath? load_folder(Geary.ConfigFile.Group config,
-                                          string key) {
-        Geary.FolderPath? path = null;
-        Gee.List<string> parts = config.get_string_list(key);
-        if (!parts.is_empty) {
-            path = Geary.AccountInformation.build_folder_path(parts);
+    private inline void save_folder(Geary.ConfigFile.Group config,
+                                    string key,
+                                    Gee.List<string>? steps) {
+        if (steps != null) {
+            config.set_string_list(key, steps);
         }
-        return path;
     }
 
 }
@@ -1274,20 +1294,20 @@ public class Accounts.AccountConfigLegacy : AccountConfig, GLib.Object {
             EMAIL_SIGNATURE_KEY, info.signature
         );
 
-        info.drafts_folder_path = Geary.AccountInformation.build_folder_path(
-            config.get_string_list(DRAFTS_FOLDER_KEY)
+        info.set_folder_steps_for_use(
+            DRAFTS, config.get_string_list(DRAFTS_FOLDER_KEY)
         );
-        info.sent_folder_path = Geary.AccountInformation.build_folder_path(
-            config.get_string_list(SENT_MAIL_FOLDER_KEY)
+        info.set_folder_steps_for_use(
+            SENT, config.get_string_list(SENT_MAIL_FOLDER_KEY)
         );
-        info.junk_folder_path = Geary.AccountInformation.build_folder_path(
-            config.get_string_list(SPAM_FOLDER_KEY)
+        info.set_folder_steps_for_use(
+            JUNK, config.get_string_list(SPAM_FOLDER_KEY)
         );
-        info.trash_folder_path = Geary.AccountInformation.build_folder_path(
-            config.get_string_list(TRASH_FOLDER_KEY)
+        info.set_folder_steps_for_use(
+            TRASH, config.get_string_list(TRASH_FOLDER_KEY)
         );
-        info.archive_folder_path = Geary.AccountInformation.build_folder_path(
-            config.get_string_list(ARCHIVE_FOLDER_KEY)
+        info.set_folder_steps_for_use(
+            ARCHIVE, config.get_string_list(ARCHIVE_FOLDER_KEY)
         );
 
         info.save_drafts = config.get_bool(SAVE_DRAFTS_KEY, true);
@@ -1322,36 +1342,34 @@ public class Accounts.AccountConfigLegacy : AccountConfig, GLib.Object {
             );
         }
 
-        Gee.ArrayList<string> empty = new Gee.ArrayList<string>();
+        Gee.List<string> empty = new Gee.ArrayList<string>();
+        Gee.List<string>? steps = null;
+
+        steps = info.get_folder_steps_for_use(DRAFTS);
         config.set_string_list(
             DRAFTS_FOLDER_KEY,
-            (info.drafts_folder_path != null
-             ? new Gee.ArrayList<string>.wrap(info.drafts_folder_path.as_array())
-             : empty)
+            steps != null ? steps : empty
         );
+
+        steps = info.get_folder_steps_for_use(SENT);
         config.set_string_list(
             SENT_MAIL_FOLDER_KEY,
-            (info.sent_folder_path != null
-             ? new Gee.ArrayList<string>.wrap(info.sent_folder_path.as_array())
-             : empty)
+            steps != null ? steps : empty
         );
+        steps = info.get_folder_steps_for_use(JUNK);
         config.set_string_list(
             SPAM_FOLDER_KEY,
-            (info.junk_folder_path != null
-             ? new Gee.ArrayList<string>.wrap(info.junk_folder_path.as_array())
-             : empty)
+            steps != null ? steps : empty
         );
+        steps = info.get_folder_steps_for_use(TRASH);
         config.set_string_list(
             TRASH_FOLDER_KEY,
-            (info.trash_folder_path != null
-             ? new Gee.ArrayList<string>.wrap(info.trash_folder_path.as_array())
-             : empty)
+            steps != null ? steps : empty
         );
+        steps = info.get_folder_steps_for_use(ARCHIVE);
         config.set_string_list(
             ARCHIVE_FOLDER_KEY,
-            (info.archive_folder_path != null
-             ? new Gee.ArrayList<string>.wrap(info.archive_folder_path.as_array())
-             : empty)
+            steps != null ? steps : empty
         );
 
         config.set_bool(SAVE_DRAFTS_KEY, info.save_drafts);
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 9aa618dc..64a5b188 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -25,17 +25,6 @@ public class Geary.AccountInformation : BaseObject {
         return a.display_name.collate(b.display_name);
     }
 
-    public static Geary.FolderPath? build_folder_path(Gee.List<string>? parts) {
-        if (parts == null || parts.size == 0)
-            return null;
-
-        Geary.FolderPath path = new Imap.FolderRoot("#geary-config");
-        foreach (string basename in parts) {
-            path = path.get_child(basename);
-        }
-        return path;
-    }
-
 
     /** A unique (engine-wide), opaque identifier for the account. */
     public string id { get; private set; }
@@ -174,21 +163,6 @@ public class Geary.AccountInformation : BaseObject {
     /** Specifies the email sig to be appended to new messages. */
     public string signature { get; set; default = ""; }
 
-    /** Draft special folder path. */
-    public Geary.FolderPath? drafts_folder_path { get; set; default = null; }
-
-    /** Sent special folder path. */
-    public Geary.FolderPath? sent_folder_path { get; set; default = null; }
-
-    /** Junk special folder path. */
-    public Geary.FolderPath? junk_folder_path { get; set; default = null; }
-
-    /** Trash special folder path. */
-    public Geary.FolderPath? trash_folder_path { get; set; default = null; }
-
-    /** Archive special folder path. */
-    public Geary.FolderPath? archive_folder_path { get; set; default = null; }
-
     /**
      * Location of the account's config directory.
      *
@@ -205,6 +179,9 @@ public class Geary.AccountInformation : BaseObject {
      */
     public File? data_dir { get; private set; default = null; }
 
+    private Gee.Map<Folder.SpecialUse?,Gee.List<string>> special_use_paths =
+        new Gee.HashMap<Folder.SpecialUse?,Gee.List<string>>();
+
     private Gee.List<Geary.RFC822.MailboxAddress> mailboxes {
         get; private set;
         default = new Gee.LinkedList<Geary.RFC822.MailboxAddress>();
@@ -287,11 +264,7 @@ public class Geary.AccountInformation : BaseObject {
         this.incoming = new ServiceInformation.copy(other.incoming);
         this.outgoing = new ServiceInformation.copy(other.outgoing);
 
-        this.drafts_folder_path = other.drafts_folder_path;
-        this.sent_folder_path = other.sent_folder_path;
-        this.junk_folder_path = other.junk_folder_path;
-        this.trash_folder_path = other.trash_folder_path;
-        this.archive_folder_path = other.archive_folder_path;
+        this.special_use_paths.set_all(other.special_use_paths);
 
         this.config_dir = other.config_dir;
         this.data_dir = other.data_dir;
@@ -368,80 +341,79 @@ public class Geary.AccountInformation : BaseObject {
         this.mailboxes.set(index, mailbox);
     }
 
-     /**
-     * Returns the configured path for a special folder use.
-     *
-     * This is used when Geary has found or created a special folder
-     * for this account. The path will be null if Geary has always
-     * been told about the special folders by the server, and hasn't
-     * had to go looking for them.  Only the ARCHIVE, DRAFTS, SENT,
-     * JUNK, and TRASH special folder types are valid to pass to this
-     * function.
+    /**
+     * Returns the folder path steps configured for a specific use.
      */
-    public Geary.FolderPath? get_special_folder_path(Folder.SpecialUse special) {
-        switch (special) {
-        case DRAFTS:
-            return this.drafts_folder_path;
-
-        case SENT:
-            return this.sent_folder_path;
-
-        case JUNK:
-            return this.junk_folder_path;
-
-        case TRASH:
-            return this.trash_folder_path;
-
-        case ARCHIVE:
-            return this.archive_folder_path;
+    public Gee.List<string> get_folder_steps_for_use(Folder.SpecialUse use) {
+        var steps = this.special_use_paths.get(use);
+        if (steps != null) {
+            steps = steps.read_only_view;
+        } else {
+            steps = Gee.List.empty();
         }
-
-        return null;
+        return steps;
     }
 
     /**
-     * Sets the configured path for a special folder type.
-     *
-     * This is only obeyed if the server doesn't tell Geary which
-     * folders are special. Only the DRAFTS, SENT, JUNK, TRASH and
-     * ARCHIVE special folder types are valid to pass to this
-     * function.
+     * Sets the configured folder path steps for a specific use.
      */
-    public void set_special_folder_path(Folder.SpecialUse special,
-                                        FolderPath? new_path) {
-        Geary.FolderPath? old_path = null;
-        switch (special) {
-        case DRAFTS:
-            old_path = this.drafts_folder_path;
-            this.drafts_folder_path = new_path;
-            break;
-
-        case SENT:
-            old_path = this.sent_folder_path;
-            this.sent_folder_path = new_path;
-            break;
-
-        case JUNK:
-            old_path = this.junk_folder_path;
-            this.junk_folder_path = new_path;
-            break;
-
-        case TRASH:
-            old_path = this.trash_folder_path;
-            this.trash_folder_path = new_path;
-            break;
+    public void set_folder_steps_for_use(Folder.SpecialUse special,
+                                         Gee.List<string>? new_path) {
+        var existing = this.special_use_paths.get(special);
+        if (new_path != null && !new_path.is_empty) {
+            this.special_use_paths.set(special, new_path);
+        } else {
+            this.special_use_paths.unset(special);
+        }
+        if ((existing == null && new_path != null) ||
+            (existing != null && new_path == null) ||
+            (existing != null &&
+             (existing.size != new_path.size ||
+              existing.contains_all(new_path)))) {
+            changed();
+        }
+    }
 
-        case ARCHIVE:
-            old_path = this.archive_folder_path;
-            this.archive_folder_path = new_path;
-            break;
+    /**
+     * Returns a folder path based on the configured steps for a use.
+     */
+    public FolderPath? new_folder_path_for_use(FolderRoot root,
+                                               Folder.SpecialUse use) {
+        FolderPath? path = null;
+        var steps = this.special_use_paths.get(use);
+        if (steps != null) {
+            path = root;
+            foreach (string step in steps) {
+                path = path.get_child(step);
+            }
         }
+        return path;
+    }
 
-        if ((old_path == null && new_path != null) ||
-            (old_path != null && new_path == null) ||
-            (old_path != null && !old_path.equal_to(new_path))) {
-            changed();
+    /**
+     * Returns the configured special folder use for a given path.
+     */
+    public Folder.SpecialUse get_folder_use_for_path(FolderPath path) {
+        var path_steps = path.as_array();
+        var use = Folder.SpecialUse.NONE;
+        foreach (var entry in this.special_use_paths.entries) {
+            var use_steps = entry.value;
+            var found = false;
+            if (path_steps.length == use_steps.size) {
+                found = true;
+                for (int i = path_steps.length - 1; i >= 0; i--) {
+                    if (path_steps[i] != use_steps[i]) {
+                        found = false;
+                        break;
+                    }
+                }
+            }
+            if (found) {
+                use = entry.key;
+                break;
+            }
         }
+        return use;
     }
 
     /**
@@ -539,11 +511,8 @@ public class Geary.AccountInformation : BaseObject {
                 this.signature == other.signature &&
                 this.incoming.equal_to(other.incoming) &&
                 this.outgoing.equal_to(other.outgoing) &&
-                this.drafts_folder_path == other.drafts_folder_path &&
-                this.sent_folder_path == other.sent_folder_path &&
-                this.junk_folder_path == other.junk_folder_path &&
-                this.trash_folder_path == other.trash_folder_path &&
-                this.archive_folder_path == other.archive_folder_path &&
+                this.special_use_paths.size == other.special_use_paths.size &&
+                this.special_use_paths.has_all(other.special_use_paths) &&
                 this.config_dir == other.config_dir &&
                 this.data_dir == other.data_dir
             )
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 2ca9c39d..d036d564 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -654,18 +654,16 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         throws GLib.Error {
         Folder? special = get_special_folder(use);
         if (special == null) {
-            FolderPath? path = information.get_special_folder_path(use);
-            if (path != null) {
-                if (!remote.is_folder_path_valid(path)) {
-                    warning(
-                        "Ignoring bad special folder path '%s' for type %s",
-                        path.to_string(),
-                        use.to_string()
-                    );
-                    path = null;
-                } else {
-                    path = this.local.imap_folder_root.copy(path);
-                }
+            FolderPath? path = information.new_folder_path_for_use(
+                this.local.imap_folder_root, use
+            );
+            if (path != null && !remote.is_folder_path_valid(path)) {
+                warning(
+                    "Ignoring bad special folder path '%s' for type %s",
+                    path.to_string(),
+                    use.to_string()
+                );
+                path = null;
             }
 
             if (path == null) {
@@ -691,7 +689,9 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
                 debug("Guessed folder \'%s\' for special_path %s",
                       path.to_string(), use.to_string()
                 );
-                information.set_special_folder_path(use, path);
+                information.set_folder_steps_for_use(
+                    use, new Gee.ArrayList<string>.wrap(path.as_array())
+                );
             }
 
             if (!this.folder_map.has_key(path)) {
diff --git a/test/client/accounts/accounts-manager-test.vala b/test/client/accounts/accounts-manager-test.vala
index 0495133f..9f2d57ca 100644
--- a/test/client/accounts/accounts-manager-test.vala
+++ b/test/client/accounts/accounts-manager-test.vala
@@ -40,15 +40,9 @@ class Accounts.ManagerTest : TestCase {
         // engine without creating all of these dirs.
 
         this.tmp = GLib.File.new_for_path(
-            GLib.DirUtils.make_tmp("geary-engine-test-XXXXXX")
+            GLib.DirUtils.make_tmp("accounts-manager-test-XXXXXX")
         );
 
-        GLib.File config = this.tmp.get_child("config");
-        config.make_directory();
-
-        GLib.File data = this.tmp.get_child("data");
-        data.make_directory();
-
         this.primary_mailbox = new Geary.RFC822.MailboxAddress(
             null, "test1 example com"
         );
@@ -60,7 +54,7 @@ class Accounts.ManagerTest : TestCase {
             this.mediator,
             this.primary_mailbox
         );
-        this.test = new Manager(this.mediator, config, data);
+        this.test = new Manager(this.mediator, this.tmp, this.tmp);
     }
 
     public override void tear_down() throws GLib.Error {
@@ -169,12 +163,13 @@ class Accounts.ManagerTest : TestCase {
         this.account.save_sent = false;
         this.account.signature = "blarg";
         this.account.use_signature = false;
-        Accounts.AccountConfigV1 config = new Accounts.AccountConfigV1(false);
 
         Geary.ConfigFile file =
-            new Geary.ConfigFile(this.tmp.get_child("config"));
+            new Geary.ConfigFile(this.tmp.get_child("config.ini"));
 
+        Accounts.AccountConfigV1 config = new Accounts.AccountConfigV1(false);
         config.save(this.account, file);
+
         Geary.AccountInformation copy = config.load(
             file, TEST_ID, this.mediator, null, null
         );
@@ -194,7 +189,7 @@ class Accounts.ManagerTest : TestCase {
             new Accounts.AccountConfigLegacy();
 
         Geary.ConfigFile file =
-            new Geary.ConfigFile(this.tmp.get_child("config"));
+            new Geary.ConfigFile(this.tmp.get_child("config.ini"));
 
         config.save(this.account, file);
         Geary.AccountInformation copy = config.load(
@@ -221,7 +216,7 @@ class Accounts.ManagerTest : TestCase {
             Geary.Credentials.Requirement.NONE;
         Accounts.ServiceConfigV1 config = new Accounts.ServiceConfigV1();
         Geary.ConfigFile file =
-            new Geary.ConfigFile(this.tmp.get_child("config"));
+            new Geary.ConfigFile(this.tmp.get_child("config.ini"));
 
         config.save(this.account, this.account.outgoing, file);
         config.load(file, copy, copy.outgoing);
@@ -246,7 +241,7 @@ class Accounts.ManagerTest : TestCase {
             Geary.Credentials.Requirement.NONE;
         Accounts.ServiceConfigLegacy config = new Accounts.ServiceConfigLegacy();
         Geary.ConfigFile file =
-            new Geary.ConfigFile(this.tmp.get_child("config"));
+            new Geary.ConfigFile(this.tmp.get_child("config.ini"));
 
         config.save(this.account, this.account.outgoing, file);
         config.load(file, copy, copy.outgoing);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]