[geary/wip/714104-refine-account-dialog] Check for conflicting config and data dirs when generating an account id

[Michael Gratton <mjog src gnome org>]

commit 3049837973bca5482f5af5e4d979a69e2928ae11
Author: Michael Gratton <mike vee net>
Date:   Wed Dec 26 15:42:40 2018 +1030

    Check for conflicting config and data dirs when generating an account id

 src/client/accounts/accounts-editor-add-pane.vala |  5 +-
 src/client/accounts/accounts-manager.vala         | 57 +++++++++++++++-------
 test/client/accounts/accounts-manager-test.vala   | 58 +++++++++++++++++------
 3 files changed, 88 insertions(+), 32 deletions(-)
---
diff --git a/src/client/accounts/accounts-editor-add-pane.vala 
b/src/client/accounts/accounts-editor-add-pane.vala
index 196dae8c..cee7d989 100644
--- a/src/client/accounts/accounts-editor-add-pane.vala
+++ b/src/client/accounts/accounts-editor-add-pane.vala
@@ -164,12 +164,13 @@ internal class Accounts.EditorAddPane : Gtk.Grid, EditorPane {
         Gtk.Widget? to_focus = null;
 
         Geary.AccountInformation account =
-            this.accounts.new_orphan_account(
+            yield this.accounts.new_orphan_account(
                 this.provider,
                 new Geary.RFC822.MailboxAddress(
                     this.real_name.value.text.strip(),
                     this.email.value.text.strip()
-                )
+                ),
+                cancellable
             );
 
         account.incoming = new_imap_service();
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index 1b3632d0..e5af1bf1 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -192,23 +192,11 @@ public class Accounts.Manager : GLib.Object {
     /**
      * Returns a new account, not yet stored on disk.
      */
-    public Geary.AccountInformation
+    public async Geary.AccountInformation
         new_orphan_account(Geary.ServiceProvider provider,
-                           Geary.RFC822.MailboxAddress primary_mailbox) {
-        string? last_account = this.accounts.keys.fold<string?>((next, last) => {
-                string? result = last;
-                if (next.has_prefix(LOCAL_ID_PREFIX)) {
-                    result = (last == null || strcmp(last, next) < 0) ? next : last;
-                }
-                return result;
-            },
-            null);
-        uint next_id = 1;
-        if (last_account != null) {
-            next_id = int.parse(last_account.substring(LOCAL_ID_PREFIX.length)) + 1;
-        }
-        string id = LOCAL_ID_FORMAT.printf(next_id);
-
+                           Geary.RFC822.MailboxAddress primary_mailbox,
+                           GLib.Cancellable? cancellable) {
+        string id = yield next_id(cancellable);
         return new Geary.AccountInformation(
             id, provider, this.local_mediator, primary_mailbox
         );
@@ -419,6 +407,43 @@ public class Accounts.Manager : GLib.Object {
         );
     }
 
+    /** Returns the next id for a new local account. */
+    private async string next_id(GLib.Cancellable? cancellable) {
+        // Get the next known free id
+        string? last_account = this.accounts.keys.fold<string?>((next, last) => {
+                string? result = last;
+                if (next.has_prefix(LOCAL_ID_PREFIX)) {
+                    result = (last == null || strcmp(last, next) < 0) ? next : last;
+                }
+                return result;
+            },
+            null);
+
+        uint next_id = 1;
+        if (last_account != null) {
+            next_id = int.parse(
+                last_account.substring(LOCAL_ID_PREFIX.length)
+            ) + 1;
+        }
+
+        // Check for existing directories that might conflict
+        string id = LOCAL_ID_FORMAT.printf(next_id);
+        try {
+            while ((yield Geary.Files.query_exists_async(
+                        this.config_dir.get_child(id), cancellable)) ||
+                   (yield Geary.Files.query_exists_async(
+                       this.data_dir.get_child(id), cancellable))) {
+                next_id++;
+                id = LOCAL_ID_FORMAT.printf(next_id);
+            }
+        } catch (GLib.Error err) {
+            // Not much we can do here except keep going anyway?
+            debug("Error checking for a free id on disk: %s", err.message);
+        }
+
+        return id;
+    }
+
     /**
      * Loads an account info from a config directory.
      *
diff --git a/test/client/accounts/accounts-manager-test.vala b/test/client/accounts/accounts-manager-test.vala
index f15752ee..6f64628d 100644
--- a/test/client/accounts/accounts-manager-test.vala
+++ b/test/client/accounts/accounts-manager-test.vala
@@ -21,7 +21,14 @@ class Accounts.ManagerTest : TestCase {
         base("AccountManagerTest");
         add_test("create_account", create_account);
         add_test("create_orphan_account", create_orphan_account);
-        add_test("create_orphan_account_with_legacy", create_orphan_account_with_legacy);
+        add_test(
+            "create_orphan_account_with_legacy",
+            create_orphan_account_with_legacy
+        );
+        add_test(
+            "create_orphan_account_with_existing_dirs",
+            create_orphan_account_with_existing_dirs
+        );
         add_test("account_config_v1", account_config_v1);
         add_test("account_config_legacy", account_config_legacy);
         add_test("service_config_v1", service_config_v1);
@@ -86,10 +93,12 @@ class Accounts.ManagerTest : TestCase {
     }
 
     public void create_orphan_account() throws GLib.Error {
-        Geary.AccountInformation account1 = this.test.new_orphan_account(
-            Geary.ServiceProvider.OTHER,
-            new Geary.RFC822.MailboxAddress(null, "test1 example com")
+        this.test.new_orphan_account.begin(
+            Geary.ServiceProvider.OTHER, this.primary_mailbox, null,
+            (obj, res) => { async_complete(res); }
         );
+        Geary.AccountInformation account1 =
+            this.test.new_orphan_account.end(async_result());
         assert(account1.id == "account_01");
 
         this.test.create_account.begin(
@@ -98,24 +107,28 @@ class Accounts.ManagerTest : TestCase {
         );
         this.test.create_account.end(async_result());
 
-        Geary.AccountInformation account2 = this.test.new_orphan_account(
-            Geary.ServiceProvider.OTHER,
-            new Geary.RFC822.MailboxAddress(null, "test1 example com")
+        this.test.new_orphan_account.begin(
+            Geary.ServiceProvider.OTHER, this.primary_mailbox, null,
+            (obj, res) => { async_complete(res); }
         );
+        Geary.AccountInformation account2 =
+            this.test.new_orphan_account.end(async_result());
         assert(account2.id == "account_02");
     }
 
     public void create_orphan_account_with_legacy() throws GLib.Error {
         this.test.create_account.begin(
             account, new GLib.Cancellable(),
-             (obj, res) => { async_complete(res); }
+            (obj, res) => { async_complete(res); }
         );
         this.test.create_account.end(async_result());
 
-        Geary.AccountInformation account1 = this.test.new_orphan_account(
-            Geary.ServiceProvider.OTHER,
-            new Geary.RFC822.MailboxAddress(null, "test1 example com")
+        this.test.new_orphan_account.begin(
+            Geary.ServiceProvider.OTHER, this.primary_mailbox, null,
+            (obj, res) => { async_complete(res); }
         );
+        Geary.AccountInformation account1 =
+            this.test.new_orphan_account.end(async_result());
         assert(account1.id == "account_01");
 
         this.test.create_account.begin(
@@ -124,13 +137,30 @@ class Accounts.ManagerTest : TestCase {
         );
         this.test.create_account.end(async_result());
 
-        Geary.AccountInformation account2 = this.test.new_orphan_account(
-            Geary.ServiceProvider.OTHER,
-            new Geary.RFC822.MailboxAddress(null, "test1 example com")
+        this.test.new_orphan_account.begin(
+            Geary.ServiceProvider.OTHER, this.primary_mailbox, null,
+            (obj, res) => { async_complete(res); }
         );
+        Geary.AccountInformation account2 =
+            this.test.new_orphan_account.end(async_result());
         assert(account2.id == "account_02");
     }
 
+    public void create_orphan_account_with_existing_dirs() throws GLib.Error {
+        GLib.File existing = this.test.config_dir.get_child("account_01");
+        existing.make_directory();
+        existing = this.test.data_dir.get_child("account_02");
+        existing.make_directory();
+
+        this.test.new_orphan_account.begin(
+            Geary.ServiceProvider.OTHER, this.primary_mailbox, null,
+            (obj, res) => { async_complete(res); }
+        );
+        Geary.AccountInformation account =
+            this.test.new_orphan_account.end(async_result());
+        assert(account.id == "account_03");
+    }
+
     public void account_config_v1() throws GLib.Error {
         this.account.label = "test-name";
         this.account.ordinal = 100;