nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Clean up Geary.Engine account API and implementation

Browse source 
Keep a single ordered list of accounts around, construct accounts
when their config is first added, and prefer accessing accounts by
config rather than id.
This commit is contained in:
Michael Gratton 2019-11-14 14:44:30 +11:00 • committed by Michael James Gratton
parent 93d1ab684b
commit ecfd772c07
5 changed files with 200 additions and 186 deletions
Download patch file Download diff file Expand all files Collapse all files

22
src/client/application/application-client.vala
View file

@ -801,14 +801,13 @@ public class Application.Client : Gtk.Application {
window.focus_in_event.connect(on_main_window_focus_in);
if (select_first_inbox) {
try {
var config = this.controller.get_first_account();
if (config != null) {
var first = this.engine.get_account_instance(config);
if (first != null) {
Geary.Folder? inbox = first.get_special_folder(INBOX);
if (inbox != null) {
window.select_folder.begin(inbox, true);
}
Geary.Account first = Geary.Collection.get_first(
this.engine.get_accounts()
);
if (first != null) {
Geary.Folder? inbox = first.get_special_folder(INBOX);
if (inbox != null) {
window.select_folder.begin(inbox, true);
}
}
} catch (GLib.Error error) {
@ -970,12 +969,9 @@ public class Application.Client : Gtk.Application {
private Geary.Folder? get_folder_from_action_target(GLib.Variant target) {
Geary.Folder? folder = null;
string account_id = (string) target.get_child_value(0);
string id = (string) target.get_child_value(0);
try {
Geary.AccountInformation? account_config =
this.engine.get_account(account_id);
Geary.Account? account =
this.engine.get_account_instance(account_config);
Geary.Account account = this.engine.get_account_for_id(id);
Geary.FolderPath? path =
account.to_folder_path(
target.get_child_value(1).get_variant()

15
src/client/application/application-controller.vala
View file

@ -1695,9 +1695,14 @@ internal class Application.Controller : Geary.BaseObject {
private void on_account_available(Geary.AccountInformation info) {
Geary.Account? account = null;
try {
account = Geary.Engine.instance.get_account_instance(info);
} catch (Error e) {
error("Error creating account instance: %s", e.message);
account = this.application.engine.get_account(info);
} catch (GLib.Error error) {
report_problem(new Geary.ProblemReport(error));
warning(
"Error creating account %s instance: %s",
info.id,
error.message
);
}
if (account != null) {
@ -1720,7 +1725,7 @@ internal class Application.Controller : Geary.BaseObject {
Accounts.Manager.Status status) {
switch (status) {
case Accounts.Manager.Status.ENABLED:
if (!this.application.engine.has_account(changed.id)) {
if (!this.application.engine.has_account(changed)) {
try {
this.application.engine.add_account(changed);
} catch (GLib.Error err) {
@ -1731,7 +1736,7 @@ internal class Application.Controller : Geary.BaseObject {
case Accounts.Manager.Status.UNAVAILABLE:
case Accounts.Manager.Status.DISABLED:
if (this.application.engine.has_account(changed.id)) {
if (this.application.engine.has_account(changed)) {
this.close_account.begin(
changed,
false,

18
src/client/composer/composer-widget.vala
View file

@ -413,7 +413,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
}
}
private Gee.Map<string, Geary.AccountInformation> accounts;
private Gee.Collection<Geary.Account> accounts;
private string body_html = "";
@ -478,7 +478,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
try {
this.accounts = this.application.engine.get_accounts();
} catch (Error e) {
} catch (GLib.Error e) {
warning("Could not fetch account info: %s", e.message);
}
@ -2379,7 +2379,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
// show nothing.
if (this.accounts.size < 1 ||
(this.accounts.size == 1 &&
!Geary.traverse<Geary.AccountInformation>(this.accounts.values).first().has_sender_aliases)) {
!Geary.traverse<Geary.Account>(this.accounts).first().information.has_sender_aliases)) {
return false;
}
@ -2397,13 +2397,11 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface {
// is set to true if the current message's from address has
// been set in the ComboBox.
bool set_active = add_account_emails_to_from_list(this.account);
foreach (Geary.AccountInformation info in this.accounts.values) {
try {
Geary.Account a = this.application.engine.get_account_instance(info);
if (a != this.account)
set_active = add_account_emails_to_from_list(a, set_active);
} catch (Error e) {
debug("Error getting account in composer: %s", e.message);
foreach (var account in this.accounts) {
if (account != this.account) {
set_active = add_account_emails_to_from_list(
account, set_active
);
}
}

310
src/engine/api/geary-engine.vala
View file

@ -49,16 +49,20 @@ public class Geary.Engine : BaseObject {
/** Determines if any accounts have been added to this instance. */
public bool has_accounts {
get { return this.accounts != null && !this.accounts.is_empty; }
get { return this.is_open && !this.accounts.is_empty; }
}
/** Determines the number of accounts added to this instance. */
public uint accounts_count {
get { return this.accounts.size; }
}
/** Location of the directory containing shared resource files. */
public File? resource_dir { get; private set; default = null; }
private Gee.HashMap<string, AccountInformation>? accounts = null;
private Gee.HashMap<string, Account>? account_instances = null;
private bool is_initialized = false;
private bool is_open = false;
private Gee.List<Account> accounts = new Gee.ArrayList<Account>();
// Would use a `weak Endpoint` value type for this map instead of
// the custom class, but we can't currently reassign built-in
@ -127,19 +131,16 @@ public class Geary.Engine : BaseObject {
public async void open_async(GLib.File resource_dir,
GLib.Cancellable? cancellable = null)
throws GLib.Error {
// initialize *before* opening the Engine ... all initialize code should assume the Engine
// is closed
// initialize *before* opening the Engine ... all initialize
// code should assume the Engine is closed
initialize_library();
if (is_open)
throw new EngineError.ALREADY_OPEN("Geary.Engine instance already open");
if (is_open) {
throw new EngineError.ALREADY_OPEN("Already open");
}
this.resource_dir = resource_dir;
accounts = new Gee.HashMap<string, AccountInformation>();
account_instances = new Gee.HashMap<string, Account>();
is_open = true;
this.is_open = true;
opened();
}
@ -147,58 +148,159 @@ public class Geary.Engine : BaseObject {
/**
* Uninitializes the engine, and removes all known accounts.
*/
public async void close_async(Cancellable? cancellable = null) throws Error {
if (!is_open)
return;
public async void close_async(Cancellable? cancellable = null)
throws GLib.Error {
if (is_open) {
foreach (var account in this.accounts) {
account_unavailable(account.information);
}
this.accounts.clear();
this.resource_dir = null;
this.is_open = false;
Gee.Collection<AccountInformation> unavailable_accounts = accounts.values;
accounts.clear();
foreach(AccountInformation account in unavailable_accounts)
account_unavailable(account);
resource_dir = null;
accounts = null;
account_instances = null;
is_open = false;
closed();
closed();
}
}
/**
* Determines if an account with a specific id has added.
* Determines if an account with a specific configuration has been added.
*/
public bool has_account(string id) {
return (this.accounts != null && this.accounts.has_key(id));
public bool has_account(AccountInformation config) {
return this.accounts.any_match(account => account.information == config);
}
/**
* Returns a current account given its id.
* Returns the account for the given configuration, if present.
*
* Throws an error if the engine has not been opened or if the
* requested account does not exist.
*/
public AccountInformation get_account(string id) throws Error {
public Account get_account(AccountInformation config) throws GLib.Error {
check_opened();
AccountInformation? info = accounts.get(id);
if (info == null) {
throw new EngineError.NOT_FOUND("No such account: %s", id);
Account? account = this.accounts.first_match(
account => account.information == config
);
if (account == null) {
throw new EngineError.NOT_FOUND("No such account");
}
return info;
return account;
}
/**
* Returns the current accounts list as a map keyed by account id.
* Returns the account for the given configuration id, if present.
*
* Throws an error if the engine has not been opened or if the
* requested account does not exist.
*/
public Account get_account_for_id(string id) throws GLib.Error {
check_opened();
Account? account = this.accounts.first_match(
account => account.information.id == id
);
if (account == null) {
throw new EngineError.NOT_FOUND("No such account");
}
return account;
}
/**
* Returns a read-only collection of current accounts.
*
* The collection is guaranteed to be ordered by {@link
* AccountInformation.compare_ascending}.
*
* Throws an error if the engine has not been opened.
*/
public Gee.Map<string, AccountInformation> get_accounts() throws Error {
public Gee.Collection<Account> get_accounts() throws GLib.Error {
check_opened();
return accounts.read_only_view;
}
/**
* Adds the account to the engine.
*
* The account will not be automatically opened, this must be done
* once added.
*/
public void add_account(AccountInformation config) throws GLib.Error {
check_opened();
if (has_account(config)) {
throw new EngineError.ALREADY_EXISTS("Account already exists");
}
ImapDB.Account local = new ImapDB.Account(
config,
config.data_dir,
this.resource_dir.get_child("sql")
);
Endpoint incoming_remote = get_shared_endpoint(
config.service_provider, config.incoming
);
Endpoint outgoing_remote = get_shared_endpoint(
config.service_provider, config.outgoing
);
Geary.Account account;
switch (config.service_provider) {
case ServiceProvider.GMAIL:
account = new ImapEngine.GmailAccount(
config, local, incoming_remote, outgoing_remote
);
break;
case ServiceProvider.YAHOO:
account = new ImapEngine.YahooAccount(
config, local, incoming_remote, outgoing_remote
);
break;
case ServiceProvider.OUTLOOK:
account = new ImapEngine.OutlookAccount(
config, local, incoming_remote, outgoing_remote
);
break;
case ServiceProvider.OTHER:
account = new ImapEngine.OtherAccount(
config, local, incoming_remote, outgoing_remote
);
break;
default:
assert_not_reached();
}
config.notify["ordinal"].connect(on_account_ordinal_changed);
this.accounts.add(account);
sort_accounts();
account_available(config);
}
/**
* Removes an account from the engine.
*
* The account must be closed before removing it.
*/
public void remove_account(AccountInformation config) throws GLib.Error {
check_opened();
Account account = get_account(config);
if (account.is_open()) {
throw new EngineError.CLOSE_REQUIRED(
"Account must be closed before removal"
);
}
config.notify["ordinal"].disconnect(on_account_ordinal_changed);
this.accounts.remove(account);
account_unavailable(config);
}
/**
* Determines if an account's IMAP service can be connected to.
*/
@ -300,105 +402,6 @@ public class Geary.Engine : BaseObject {
}
}
/**
* Creates a Geary.Account from a Geary.AccountInformation (which is what
* other methods in this interface deal in).
*/
public Geary.Account get_account_instance(AccountInformation config)
throws Error {
check_opened();
if (account_instances.has_key(config.id))
return account_instances.get(config.id);
ImapDB.Account local = new ImapDB.Account(
config,
config.data_dir,
this.resource_dir.get_child("sql")
);
Endpoint incoming_remote = get_shared_endpoint(
config.service_provider, config.incoming
);
Endpoint outgoing_remote = get_shared_endpoint(
config.service_provider, config.outgoing
);
Geary.Account account;
switch (config.service_provider) {
case ServiceProvider.GMAIL:
account = new ImapEngine.GmailAccount(
config, local, incoming_remote, outgoing_remote
);
break;
case ServiceProvider.YAHOO:
account = new ImapEngine.YahooAccount(
config, local, incoming_remote, outgoing_remote
);
break;
case ServiceProvider.OUTLOOK:
account = new ImapEngine.OutlookAccount(
config, local, incoming_remote, outgoing_remote
);
break;
case ServiceProvider.OTHER:
account = new ImapEngine.OtherAccount(
config, local, incoming_remote, outgoing_remote
);
break;
default:
assert_not_reached();
}
account_instances.set(config.id, account);
return account;
}
/**
* Adds the account to be tracked by the engine.
*/
public void add_account(AccountInformation account) throws Error {
check_opened();
if (accounts.has_key(account.id)) {
throw new EngineError.ALREADY_EXISTS(
"Account id '%s' already exists", account.id
);
}
accounts.set(account.id, account);
account_available(account);
}
/**
* Removes an account from the engine.
*/
public void remove_account(AccountInformation account)
throws GLib.Error {
check_opened();
// Ensure account is closed.
if (this.account_instances.has_key(account.id) &&
this.account_instances.get(account.id).is_open()) {
throw new EngineError.CLOSE_REQUIRED(
"Account %s must be closed before removal", account.id
);
}
if (this.accounts.has_key(account.id)) {
// Send the account-unavailable signal, account will be
// removed client side.
account_unavailable(account);
// Then remove the account data from the engine.
this.accounts.unset(account.id);
this.account_instances.unset(account.id);
}
}
/**
* Changes the service configuration for an account.
*
@ -409,33 +412,28 @@ public class Geary.Engine : BaseObject {
* will also be updated so that the new configuration will start
* taking effect immediately.
*/
public async void update_account_service(AccountInformation account,
public async void update_account_service(AccountInformation config,
ServiceInformation updated,
GLib.Cancellable? cancellable)
throws GLib.Error {
Account? impl = this.account_instances.get(account.id);
if (impl == null) {
throw new EngineError.BAD_PARAMETERS(
"Account has not been added to the engine: %s", account.id
);
}
Account account = get_account(config);
ClientService? service = null;
switch (updated.protocol) {
case Protocol.IMAP:
account.incoming = updated;
service = impl.incoming;
config.incoming = updated;
service = account.incoming;
break;
case Protocol.SMTP:
account.outgoing = updated;
service = impl.outgoing;
config.outgoing = updated;
service = account.outgoing;
break;
}
Endpoint remote = get_shared_endpoint(account.service_provider, updated);
Endpoint remote = get_shared_endpoint(config.service_provider, updated);
yield service.update_configuration(updated, remote, cancellable);
account.changed();
config.changed();
}
private Geary.Endpoint get_shared_endpoint(ServiceProvider provider,
@ -489,4 +487,16 @@ public class Geary.Engine : BaseObject {
);
}
private void sort_accounts() {
this.accounts.sort((a, b) => {
return AccountInformation.compare_ascending(
a.information, b.information
);
});
}
private void on_account_ordinal_changed() {
sort_accounts();
}
}

21
test/engine/api/geary-engine-test.vala
View file

@ -58,6 +58,7 @@ class Geary.EngineTest : TestCase {
new MockCredentialsMediator(),
new RFC822.MailboxAddress(null, "test1@example.com")
);
this.account.set_account_directories(this.tmp, this.tmp);
}
public override void tear_down () {
@ -72,10 +73,10 @@ class Geary.EngineTest : TestCase {
}
public void add_account() throws GLib.Error {
assert_false(this.engine.has_account(this.account.id));
assert_false(this.engine.has_account(this.account));
this.engine.add_account(this.account);
assert_true(this.engine.has_account(this.account.id), "Account not added");
assert_true(this.engine.has_account(this.account), "Account not added");
try {
this.engine.add_account(this.account);
@ -87,23 +88,27 @@ class Geary.EngineTest : TestCase {
public void remove_account() throws GLib.Error {
this.engine.add_account(this.account);
assert_true(this.engine.has_account(this.account.id));
assert_true(this.engine.has_account(this.account));
this.engine.remove_account(this.account);
assert_false(this.engine.has_account(this.account.id), "Account not rmoeved");
assert_false(this.engine.has_account(this.account), "Account not removed");
// Should not throw an error
this.engine.remove_account(this.account);
try {
this.engine.remove_account(this.account);
assert_not_reached();
} catch (GLib.Error err) {
// expected
}
}
public void re_add_account() throws GLib.Error {
assert_false(this.engine.has_account(this.account.id));
assert_false(this.engine.has_account(this.account));
this.engine.add_account(this.account);
this.engine.remove_account(this.account);
this.engine.add_account(this.account);
assert_true(this.engine.has_account(this.account.id));
assert_true(this.engine.has_account(this.account));
}
private void delete(File parent) throws Error {