[geary/wip/714104-refine-account-dialog: 31/69] Tidy up Geary.AccountInformation creation.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/714104-refine-account-dialog: 31/69] Tidy up Geary.AccountInformation creation.
- Date: Fri, 30 Nov 2018 12:50:56 +0000 (UTC)
commit 74fa29b73f06d9f44af09255f089b4a5e5af68d5
Author: Michael James Gratton <mike vee net>
Date: Mon Jul 23 13:28:58 2018 +1000
Tidy up Geary.AccountInformation creation.
Pass service provider to ctor so we can make the service provider
property immutable. Fill in service label in ctor so it's always set to
something useful. Move creating orphans from Engine to AccountManager
since it exists only to assign an internal id for new accounts, so it
should handled by the account manager anyway.
src/client/accounts/account-manager.vala | 77 ++++++-----
src/client/accounts/add-edit-page.vala | 5 +-
src/engine/api/geary-account-information.vala | 39 ++++--
src/engine/api/geary-engine.vala | 39 ------
test/client/accounts/account-manager-test.vala | 151 +++++++++++++++++++++
.../engine/api/geary-account-information-test.vala | 5 +-
test/engine/api/geary-engine-test.vala | 87 ++----------
test/engine/app/app-conversation-monitor-test.vala | 1 +
.../engine/imap-engine/account-processor-test.vala | 1 +
test/meson.build | 9 ++
test/test-client.vala | 1 +
11 files changed, 251 insertions(+), 164 deletions(-)
---
diff --git a/src/client/accounts/account-manager.vala b/src/client/accounts/account-manager.vala
index aa10d711..05280952 100644
--- a/src/client/accounts/account-manager.vala
+++ b/src/client/accounts/account-manager.vala
@@ -69,6 +69,10 @@ errordomain AccountError {
public class AccountManager : GLib.Object {
+ private const string LOCAL_ID_PREFIX = "account_";
+ private const string LOCAL_ID_FORMAT = "account_%02u";
+ private const string GOA_ID_PREFIX = "goa_";
+
private const string ACCOUNT_CONFIG_GROUP = "AccountInformation";
private const string ACCOUNT_MANAGER_GROUP = "AccountManager";
private const string IMAP_CONFIG_GROUP = "IMAP";
@@ -94,8 +98,6 @@ public class AccountManager : GLib.Object {
private const string TRASH_FOLDER_KEY = "trash_folder";
private const string USE_EMAIL_SIGNATURE_KEY = "use_email_signature";
- private const string GOA_ID_PREFIX = "goa_";
-
/**
* Specifies the overall status of an account.
@@ -222,10 +224,37 @@ public class AccountManager : GLib.Object {
this.goa_service.account_removed.connect(on_goa_account_removed);
}
+ /**
+ * Returns a new account, not yet stored on disk.
+ */
+ public Geary.AccountInformation
+ new_orphan_account(Geary.ServiceProvider provider,
+ Geary.ServiceInformation imap,
+ Geary.ServiceInformation smtp) {
+ 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);
+
+ return new Geary.AccountInformation(id, provider, imap, smtp);
+ }
+
public LocalServiceInformation new_libsecret_service(Geary.Protocol service) {
return new LocalServiceInformation(service, libsecret);
}
+ /**
+ * Creates new account's disk and credential storage as needed.
+ */
public async void create_account(Geary.AccountInformation account,
GLib.Cancellable? cancellable)
throws GLib.Error {
@@ -757,52 +786,34 @@ public class AccountManager : GLib.Object {
smtp.load_settings(smtp_config);
}
- Geary.AccountInformation info = new Geary.AccountInformation(
- id, imap, smtp
+ return new Geary.AccountInformation(
+ id, provider, imap, smtp
);
- info.service_provider = provider;
-
- // Known providers such as GMail will have a label specified
- // by clients, but other accounts can only really be
- // identified by their server names. Try to extract a 'nice'
- // value for the label here.
- if (provider == Geary.ServiceProvider.OTHER) {
- string imap_host = imap.host;
- string[] host_parts = imap_host.split(".");
- if (host_parts.length > 1) {
- host_parts = host_parts[1:host_parts.length];
- }
- info.service_label = string.joinv(".", host_parts);
- }
-
- return info;
}
private Geary.AccountInformation new_goa_account(string id,
Goa.Object account) {
GoaMediator mediator = new GoaMediator(account);
- Geary.AccountInformation info = new Geary.AccountInformation(
- id,
- new GoaServiceInformation(Geary.Protocol.IMAP, mediator, account),
- new GoaServiceInformation(Geary.Protocol.SMTP, mediator, account)
- );
-
- info.service_label = account.get_account().provider_name;
+ Geary.ServiceProvider provider = Geary.ServiceProvider.OTHER;
switch (account.get_account().provider_type) {
case "google":
- info.service_provider = Geary.ServiceProvider.GMAIL;
+ provider = Geary.ServiceProvider.GMAIL;
break;
case "windows_live":
- info.service_provider = Geary.ServiceProvider.OUTLOOK;
- break;
-
- default:
- info.service_provider = Geary.ServiceProvider.OTHER;
+ provider = Geary.ServiceProvider.OUTLOOK;
break;
}
+ Geary.AccountInformation info = new Geary.AccountInformation(
+ id,
+ provider,
+ new GoaServiceInformation(Geary.Protocol.IMAP, mediator, account),
+ new GoaServiceInformation(Geary.Protocol.SMTP, mediator, account)
+ );
+ info.service_label = account.get_account().provider_name;
+
return info;
}
diff --git a/src/client/accounts/add-edit-page.vala b/src/client/accounts/add-edit-page.vala
index 66a05a95..21a38bb4 100644
--- a/src/client/accounts/add-edit-page.vala
+++ b/src/client/accounts/add-edit-page.vala
@@ -697,7 +697,9 @@ public class AddEditPage : Gtk.Box {
);
try {
- info = this.application.engine.create_orphan_account(imap, smtp);
+ info = this.application.controller.account_manager.new_orphan_account(
+ this.get_service_provider(), imap, smtp
+ );
} catch (Error err) {
debug("Unable to create account %s for %s: %s",
this.id, this.email_address, err.message);
@@ -717,7 +719,6 @@ public class AddEditPage : Gtk.Box {
info.smtp.credentials = smtp_credentials;
info.imap.remember_password = this.remember_password;
info.smtp.remember_password = this.remember_password;
- info.service_provider = this.get_service_provider();
info.save_sent_mail = this.save_sent_mail;
info.imap.host = this.imap_host;
info.imap.port = this.imap_port;
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 50e6a9ff..c1b35d1b 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -92,6 +92,14 @@ public class Geary.AccountInformation : BaseObject {
*/
public string id { get; private set; }
+ /** Specifies the email provider for this account. */
+ public Geary.ServiceProvider service_provider { get; private set; }
+
+ /** A human-readable label describing the email service provider. */
+ public string service_label {
+ get; public set;
+ }
+
/**
* A unique human-readable display name for this account.
*
@@ -133,16 +141,6 @@ public class Geary.AccountInformation : BaseObject {
*/
public Gee.List<Geary.RFC822.MailboxAddress>? alternate_mailboxes { get; private set; default = null; }
- /** Specifies the email provider for this account. */
- public Geary.ServiceProvider service_provider {
- get; set; default = Geary.ServiceProvider.OTHER;
- }
-
- /** A human-readable label describing the service. */
- public string service_label {
- get; set; default = "";
- }
-
public int prefetch_period_days {
get; set; default = DEFAULT_PREFETCH_PERIOD_DAYS;
}
@@ -215,18 +213,36 @@ public class Geary.AccountInformation : BaseObject {
* Creates a new, empty account info file.
*/
public AccountInformation(string id,
+ ServiceProvider provider,
ServiceInformation imap,
ServiceInformation smtp) {
this.id = id;
+ this.service_provider = provider;
this.imap = imap;
this.smtp = smtp;
+
+ // Known providers such as Gmail will have a label specified
+ // by clients, but other accounts can only really be
+ // identified by their server names. Try to extract a 'nice'
+ // value for label based on service host names.
+ string imap_host = imap.host;
+ string[] host_parts = imap_host.split(".");
+ if (host_parts.length > 1) {
+ host_parts = host_parts[1:host_parts.length];
+ }
+ this.service_label = string.joinv(".", host_parts);
}
/**
* Creates a copy of an instance.
*/
public AccountInformation.temp_copy(AccountInformation from) {
- this(from.id, from.imap.temp_copy(), from.smtp.temp_copy());
+ this(
+ from.id,
+ from.service_provider,
+ from.imap.temp_copy(),
+ from.smtp.temp_copy()
+ );
copy_from(from);
this.is_copy = true;
}
@@ -245,7 +261,6 @@ public class Geary.AccountInformation : BaseObject {
foreach (RFC822.MailboxAddress alternate_mailbox in from.alternate_mailboxes)
add_alternate_mailbox(alternate_mailbox);
}
- this.service_provider = from.service_provider;
this.prefetch_period_days = from.prefetch_period_days;
this.save_sent_mail = from.save_sent_mail;
this.ordinal = from.ordinal;
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index 3fce1cfa..6ccd1786 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -16,9 +16,6 @@
*/
public class Geary.Engine : BaseObject {
- private const string ID_PREFIX = "account_";
- private const string ID_FORMAT = "account_%02u";
-
[Flags]
public enum ValidationOption {
NONE = 0,
@@ -203,42 +200,6 @@ public class Geary.Engine : BaseObject {
return accounts.read_only_view;
}
- /**
- * Returns a new account, not yet stored on disk.
- *
- * Throws an error if the engine has not been opened or if an
- * invalid account id is generated.
- */
- public AccountInformation create_orphan_account(ServiceInformation imap,
- ServiceInformation smtp)
- throws GLib.Error {
- check_opened();
-
- // We might want to allow the client to specify the id, but
- // just generate one here for now: Use a common prefix and a
- // numeric suffix, starting at 1. To generate the next id,
- // find the last account and increment its suffix.
-
- string? last_account = this.accounts.keys.fold<string?>((next, last) => {
- string? result = last;
- if (next.has_prefix(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(ID_PREFIX.length)) + 1;
- }
- string id = ID_FORMAT.printf(next_id);
-
- if (this.accounts.has_key(id))
- throw new EngineError.ALREADY_EXISTS("Account %s already exists", id);
-
- return new AccountInformation(id, imap, smtp);
- }
-
/**
* Returns whether the account information "validates." If validate_connection is true,
* we check if we can connect to the endpoints and authenticate using the supplied credentials.
diff --git a/test/client/accounts/account-manager-test.vala b/test/client/accounts/account-manager-test.vala
new file mode 100644
index 00000000..eee44635
--- /dev/null
+++ b/test/client/accounts/account-manager-test.vala
@@ -0,0 +1,151 @@
+/*
+ * 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.
+ */
+
+class AccountManagerTest : TestCase {
+
+
+ private AccountManager? test = null;
+ private File? tmp = null;
+
+
+ public AccountManagerTest() {
+ 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);
+ }
+
+ public override void set_up() throws GLib.Error {
+ // XXX this whole thing stinks. We need to be able to test the
+ // engine without creating all of these dirs.
+
+ this.tmp = GLib.File.new_for_path(
+ GLib.DirUtils.make_tmp("geary-engine-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.test = new AccountManager(new GearyApplication(), config, data);
+ }
+
+ public override void tear_down() throws GLib.Error {
+ this.test = null;
+ @delete(this.tmp);
+ }
+
+ public void create_account() throws GLib.Error {
+ const string ID = "test";
+ Geary.AccountInformation account = new Geary.AccountInformation(
+ ID,
+ Geary.ServiceProvider.OTHER,
+ new Geary.MockServiceInformation(),
+ new Geary.MockServiceInformation()
+ );
+ bool was_added = false;
+ bool was_enabled = false;
+
+ this.test.account_added.connect((added, status) => {
+ was_added = (added == account);
+ was_enabled = (status == AccountManager.Status.ENABLED);
+ });
+
+ this.test.create_account.begin(
+ account, new GLib.Cancellable(),
+ (obj, res) => { async_complete(res); }
+ );
+ this.test.create_account.end(async_result());
+
+ assert_int(1, this.test.size, "Account manager size");
+ assert_equal(account, this.test.get_account(ID), "Is not contained");
+ assert_true(was_added, "Was not added");
+ assert_true(was_enabled, "Was not enabled");
+ }
+
+ public void create_orphan_account() throws GLib.Error {
+ Geary.AccountInformation account1 = this.test.new_orphan_account(
+ Geary.ServiceProvider.OTHER,
+ new Geary.MockServiceInformation(),
+ new Geary.MockServiceInformation()
+ );
+ assert(account1.id == "account_01");
+
+ this.test.create_account.begin(
+ account1, new GLib.Cancellable(),
+ (obj, res) => { async_complete(res); }
+ );
+ this.test.create_account.end(async_result());
+
+ Geary.AccountInformation account2 = this.test.new_orphan_account(
+ Geary.ServiceProvider.OTHER,
+ new Geary.MockServiceInformation(),
+ new Geary.MockServiceInformation()
+ );
+ assert(account2.id == "account_02");
+ }
+
+ public void create_orphan_account_with_legacy() throws GLib.Error {
+ const string ID = "test";
+ Geary.AccountInformation account = new Geary.AccountInformation(
+ ID,
+ Geary.ServiceProvider.OTHER,
+ new Geary.MockServiceInformation(),
+ new Geary.MockServiceInformation()
+ );
+
+ this.test.create_account.begin(
+ account, new GLib.Cancellable(),
+ (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.MockServiceInformation(),
+ new Geary.MockServiceInformation()
+ );
+ assert(account1.id == "account_01");
+
+ this.test.create_account.begin(
+ account1, new GLib.Cancellable(),
+ (obj, res) => { async_complete(res); }
+ );
+ this.test.create_account.end(async_result());
+
+ Geary.AccountInformation account2 = this.test.new_orphan_account(
+ Geary.ServiceProvider.OTHER,
+ new Geary.MockServiceInformation(),
+ new Geary.MockServiceInformation()
+ );
+ assert(account2.id == "account_02");
+ }
+
+ private void delete(File parent) throws Error {
+ FileInfo info = parent.query_info(
+ "standard::*",
+ FileQueryInfoFlags.NOFOLLOW_SYMLINKS
+ );
+
+ if (info.get_file_type () == FileType.DIRECTORY) {
+ FileEnumerator enumerator = parent.enumerate_children(
+ "standard::*",
+ FileQueryInfoFlags.NOFOLLOW_SYMLINKS
+ );
+
+ info = null;
+ while (((info = enumerator.next_file()) != null)) {
+ @delete(parent.get_child(info.get_name()));
+ }
+ }
+
+ parent.delete();
+ }
+
+}
diff --git a/test/engine/api/geary-account-information-test.vala
b/test/engine/api/geary-account-information-test.vala
index 7cab7bd4..5215b8de 100644
--- a/test/engine/api/geary-account-information-test.vala
+++ b/test/engine/api/geary-account-information-test.vala
@@ -15,7 +15,10 @@ class Geary.AccountInformationTest : TestCase {
public void has_email_address() throws GLib.Error {
AccountInformation test = new AccountInformation(
- "test", new MockServiceInformation(), new MockServiceInformation()
+ "test",
+ ServiceProvider.OTHER,
+ new MockServiceInformation(),
+ new MockServiceInformation()
);
test.primary_mailbox = (new RFC822.MailboxAddress(null, "test1 example com"));
diff --git a/test/engine/api/geary-engine-test.vala b/test/engine/api/geary-engine-test.vala
index ae2c9c35..8a6f4bbd 100644
--- a/test/engine/api/geary-engine-test.vala
+++ b/test/engine/api/geary-engine-test.vala
@@ -18,8 +18,6 @@ class Geary.EngineTest : TestCase {
add_test("add_account", add_account);
add_test("remove_account", remove_account);
add_test("re_add_account", re_add_account);
- add_test("create_orphan_account_with_legacy", create_orphan_account_with_legacy);
- add_test("create_orphan_account", create_orphan_account);
}
~EngineTest() {
@@ -65,7 +63,9 @@ class Geary.EngineTest : TestCase {
}
public void add_account() throws GLib.Error {
- AccountInformation info = this.engine.create_orphan_account(
+ AccountInformation info = new AccountInformation(
+ "test",
+ ServiceProvider.OTHER,
new MockServiceInformation(),
new MockServiceInformation()
);
@@ -83,7 +83,9 @@ class Geary.EngineTest : TestCase {
}
public void remove_account() throws GLib.Error {
- AccountInformation info = this.engine.create_orphan_account(
+ AccountInformation info = new AccountInformation(
+ "test",
+ ServiceProvider.OTHER,
new MockServiceInformation(),
new MockServiceInformation()
);
@@ -98,7 +100,9 @@ class Geary.EngineTest : TestCase {
}
public void re_add_account() throws GLib.Error {
- AccountInformation info = this.engine.create_orphan_account(
+ AccountInformation info = new AccountInformation(
+ "test",
+ ServiceProvider.OTHER,
new MockServiceInformation(),
new MockServiceInformation()
);
@@ -111,78 +115,7 @@ class Geary.EngineTest : TestCase {
assert_true(this.engine.has_account(info.id));
}
- public void create_orphan_account() throws Error {
- try {
- AccountInformation info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_01");
- this.engine.add_account(info);
-
- info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_02");
- this.engine.add_account(info);
-
- info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_03");
- this.engine.add_account(info);
-
- info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_04");
- } catch (Error err) {
- print("\nerr: %s\n", err.message);
- assert_not_reached();
- }
- }
-
- public void create_orphan_account_with_legacy() throws Error {
- this.engine.add_account(
- new AccountInformation(
- "foo",
- new MockServiceInformation(),
- new MockServiceInformation()
- )
- );
-
- AccountInformation info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_01");
- this.engine.add_account(info);
-
- info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_02");
-
- this.engine.add_account(
- new AccountInformation(
- "bar",
- new MockServiceInformation(),
- new MockServiceInformation()
- )
- );
-
- info = this.engine.create_orphan_account(
- new MockServiceInformation(),
- new MockServiceInformation()
- );
- assert(info.id == "account_02");
- }
-
- private void delete(File parent) throws Error {
+ private void delete(File parent) throws Error {
FileInfo info = parent.query_info(
"standard::*",
FileQueryInfoFlags.NOFOLLOW_SYMLINKS
diff --git a/test/engine/app/app-conversation-monitor-test.vala
b/test/engine/app/app-conversation-monitor-test.vala
index 285a62b4..a641a8fb 100644
--- a/test/engine/app/app-conversation-monitor-test.vala
+++ b/test/engine/app/app-conversation-monitor-test.vala
@@ -30,6 +30,7 @@ class Geary.App.ConversationMonitorTest : TestCase {
public override void set_up() {
this.account_info = new AccountInformation(
"account_01",
+ ServiceProvider.OTHER,
new MockServiceInformation(),
new MockServiceInformation()
);
diff --git a/test/engine/imap-engine/account-processor-test.vala
b/test/engine/imap-engine/account-processor-test.vala
index ebe53b79..d89483de 100644
--- a/test/engine/imap-engine/account-processor-test.vala
+++ b/test/engine/imap-engine/account-processor-test.vala
@@ -71,6 +71,7 @@ public class Geary.ImapEngine.AccountProcessorTest : TestCase {
public override void set_up() {
this.info = new Geary.AccountInformation(
"test-info",
+ ServiceProvider.OTHER,
new MockServiceInformation(),
new MockServiceInformation()
);
diff --git a/test/meson.build b/test/meson.build
index 18f91d9e..bea158a1 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -61,6 +61,15 @@ geary_test_engine_sources = [
geary_test_client_sources = [
'test-client.vala',
+ # These should be included in the test lib sources, but we can't
+ # since that would make the test lib depend on geary-engine.vapi,
+ # and the engine test sute needs to depend
+ # geary-engine_internal.vapi, which leads to duplicate symbols when
+ # linking
+ 'engine/api/geary-credentials-mediator-mock.vala',
+ 'engine/api/geary-service-information-mock.vala',
+
+ 'client/accounts/account-manager-test.vala',
'client/application/geary-configuration-test.vala',
'client/components/client-web-view-test.vala',
'client/components/client-web-view-test-case.vala',
diff --git a/test/test-client.vala b/test/test-client.vala
index 06352e89..31aa1974 100644
--- a/test/test-client.vala
+++ b/test/test-client.vala
@@ -39,6 +39,7 @@ int main(string[] args) {
// Keep this before other ClientWebView based tests since it tests
// WebContext init
+ client.add_suite(new AccountManagerTest().get_suite());
client.add_suite(new ClientWebViewTest().get_suite());
client.add_suite(new ComposerWebViewTest().get_suite());
client.add_suite(new ConfigurationTest().get_suite());
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
