From 4e0950f9bc832c56c83d357fade093320a81865c Mon Sep 17 00:00:00 2001 From: Michael James Gratton Date: Fri, 25 May 2018 00:32:17 +1000 Subject: [PATCH] Introduce a ConfigFile class to improve config management. ConfigFile is a GLib.KeyFile-like class (and is backed by a KeyFile) that nonetheless provides convenient a high-level API for getting and setting grouped config values and asynchronous loading and saving. * src/engine/util/util-config-file.vala: New ConfigFile class. * src/engine/api/geary-service-information.vala (ServiceInformation): Require ConfigFile groups rather than KeyFile instances for loading and saving. Update subclasses and unit tests. * src/client/accounts/account-manager.vala (AccountManager): Move generic account key consts here from Config. Instead of using KeyFile directly, use ConfigFile groups for loading and saving settings. Rename load and save methods to be a bit more consistent with the class's role, and make save_account() throw its errors so they can be reported to the user. Update call sites. * src/client/accounts/local-service-information.vala (LocalServiceInformation): Move service-specific config key consts here, use new ConfigFile groups for loading and saving. * src/engine/api/geary-config.vala: Removed, all config consts have been moved to the classes using them and KeyFile convenience methods subsumed by ConfigFile. --- po/POTFILES.in | 2 +- .../account-dialog-account-list-pane.vala | 11 +- src/client/accounts/account-manager.vala | 272 +++++++++++------- .../accounts/local-service-information.vala | 134 +++------ src/client/application/geary-controller.vala | 39 ++- src/engine/api/geary-config.vala | 98 ------- src/engine/api/geary-service-information.vala | 6 +- src/engine/meson.build | 2 +- src/engine/util/util-config-file.vala | 216 ++++++++++++++ .../api/geary-service-information-mock.vala | 12 +- test/engine/util-config-file-test.vala | 94 ++++++ test/meson.build | 1 + test/test-engine.vala | 1 + 13 files changed, 568 insertions(+), 320 deletions(-) delete mode 100644 src/engine/api/geary-config.vala create mode 100644 src/engine/util/util-config-file.vala create mode 100644 test/engine/util-config-file-test.vala diff --git a/po/POTFILES.in b/po/POTFILES.in index 3a52d710..ef669378 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -22,7 +22,6 @@ src/client/accounts/login-dialog.vala src/client/application/autostart-manager.vala src/client/application/geary-application.vala src/client/application/geary-args.vala -src/client/application/geary-config.vala src/client/application/geary-controller.vala src/client/application/main.vala src/client/application/secret-mediator.vala @@ -380,6 +379,7 @@ src/engine/state/state-machine.vala src/engine/state/state-mapping.vala src/engine/util/util-ascii.vala src/engine/util/util-collection.vala +src/engine/util/util-config-file.vala src/engine/util/util-connectivity-manager.vala src/engine/util/util-files.vala src/engine/util/util-generic-capabilities.vala diff --git a/src/client/accounts/account-dialog-account-list-pane.vala b/src/client/accounts/account-dialog-account-list-pane.vala index a93ea0ec..56060fdd 100644 --- a/src/client/accounts/account-dialog-account-list-pane.vala +++ b/src/client/accounts/account-dialog-account-list-pane.vala @@ -223,8 +223,15 @@ public class AccountDialogAccountListPane : AccountDialogPane { // To prevent unnecessary work, only set ordinal if there's a change. if (i != account.ordinal) { account.ordinal = i; - this.application.controller.account_manager.store_to_file.begin( - account, null + this.application.controller.account_manager.save_account.begin( + account, null, + (obj, res) => { + try { + this.application.controller.account_manager.save_account.end(res); + } catch (GLib.Error err) { + warning("Error saving account: %s", err.message); + } + } ); } } diff --git a/src/client/accounts/account-manager.vala b/src/client/accounts/account-manager.vala index e68862a4..13e56462 100644 --- a/src/client/accounts/account-manager.vala +++ b/src/client/accounts/account-manager.vala @@ -8,7 +8,7 @@ /** * Current supported credential providers. */ -public enum CredentialsProvider{ +public enum CredentialsProvider { /** Credentials are provided and stored by libsecret. */ LIBSECRET; @@ -42,6 +42,29 @@ errordomain AccountError { public class AccountManager : GLib.Object { + private const string ACCOUNT_CONFIG_GROUP = "AccountInformation"; + private const string IMAP_CONFIG_GROUP = "IMAP"; + private const string SMTP_CONFIG_GROUP = "SMTP"; + + private const string ALTERNATE_EMAILS_KEY = "alternate_emails"; + private const string ARCHIVE_FOLDER_KEY = "archive_folder"; + private const string CREDENTIALS_METHOD_KEY = "credentials_method"; + private const string CREDENTIALS_PROVIDER_KEY = "credentials_provider"; + private const string DRAFTS_FOLDER_KEY = "drafts_folder"; + private const string EMAIL_SIGNATURE_KEY = "email_signature"; + private const string NICKNAME_KEY = "nickname"; + private const string ORDINAL_KEY = "ordinal"; + private const string PREFETCH_PERIOD_DAYS_KEY = "prefetch_period_days"; + private const string PRIMARY_EMAIL_KEY = "primary_email"; + private const string REAL_NAME_KEY = "real_name"; + private const string SAVE_DRAFTS_KEY = "save_drafts"; + private const string SAVE_SENT_MAIL_KEY = "save_sent_mail"; + private const string SENT_MAIL_FOLDER_KEY = "sent_mail_folder"; + private const string SERVICE_PROVIDER_KEY = "service_provider"; + private const string SPAM_FOLDER_KEY = "spam_folder"; + private const string TRASH_FOLDER_KEY = "trash_folder"; + private const string USE_EMAIL_SIGNATURE_KEY = "use_email_signature"; + private Geary.Engine engine; private GLib.File user_config_dir; private GLib.File user_data_dir; @@ -61,7 +84,7 @@ public class AccountManager : GLib.Object { public Geary.ServiceInformation new_libsecret_service(Geary.Service service, Geary.CredentialsMethod method) { - return new LocalServiceInformation(service, libsecret); + return new LocalServiceInformation(service, method, libsecret); } @@ -104,7 +127,7 @@ public class AccountManager : GLib.Object { if (info.get_file_type() == FileType.DIRECTORY) { try { string id = info.get_name(); - this.engine.add_account(load_from_file(id)); + this.engine.add_account(yield load_account(id, cancellable)); } catch (GLib.Error err) { // XXX want to report this problem to the user // somehow, but at this point in the app's @@ -128,36 +151,44 @@ public class AccountManager : GLib.Object { * Throws an error if the config file was not found, could not be * parsed, or doesn't have all required fields. */ - public Geary.AccountInformation? load_from_file(string id) + public async Geary.AccountInformation? + load_account(string id, GLib.Cancellable? cancellable) throws Error { + GLib.File config_dir = this.user_config_dir.get_child(id); + GLib.File data_dir = this.user_data_dir.get_child(id); - File file = this.user_config_dir.get_child(id).get_child(Geary.Config.SETTINGS_FILENAME); + Geary.ConfigFile config_file = new Geary.ConfigFile( + config_dir.get_child(Geary.AccountInformation.SETTINGS_FILENAME) + ); - KeyFile key_file = new KeyFile(); - key_file.load_from_file(file.get_path() ?? "", KeyFileFlags.NONE); + yield config_file.load(cancellable); + + Geary.ConfigFile.Group config = config_file.get_group(ACCOUNT_CONFIG_GROUP); + + Geary.ConfigFile.Group imap_config = config_file.get_group(IMAP_CONFIG_GROUP); + imap_config.set_fallback(ACCOUNT_CONFIG_GROUP, "imap_"); + + Geary.ConfigFile.Group smtp_config = config_file.get_group(SMTP_CONFIG_GROUP); + smtp_config.set_fallback(ACCOUNT_CONFIG_GROUP, "smtp_"); CredentialsProvider provider = CredentialsProvider.from_string( - Geary.Config.get_string_value( - key_file, - Geary.Config.GROUP, - Geary.Config.CREDENTIALS_PROVIDER_KEY, - CredentialsProvider.LIBSECRET.to_string()) - ); - Geary.CredentialsMethod method = Geary.CredentialsMethod.from_string( - Geary.Config.get_string_value( - key_file, - Geary.Config.GROUP, - Geary.Config.CREDENTIALS_METHOD_KEY, - Geary.CredentialsMethod.PASSWORD.to_string() + config.get_string( + CREDENTIALS_PROVIDER_KEY, + CredentialsProvider.LIBSECRET.to_string() ) ); - Geary.ServiceInformation imap_information; - Geary.ServiceInformation smtp_information; + Geary.CredentialsMethod method = Geary.CredentialsMethod.from_string( + config.get_string(CREDENTIALS_METHOD_KEY, + Geary.CredentialsMethod.PASSWORD.to_string()) + ); + + Geary.ServiceInformation imap_info; + Geary.ServiceInformation smtp_info; switch (provider) { case CredentialsProvider.LIBSECRET: - imap_information = new_libsecret_service(Geary.Service.IMAP, method); - smtp_information = new_libsecret_service(Geary.Service.SMTP, method); + imap_info = new_libsecret_service(Geary.Service.IMAP, method); + smtp_info = new_libsecret_service(Geary.Service.SMTP, method); break; default: @@ -165,22 +196,23 @@ public class AccountManager : GLib.Object { } Geary.AccountInformation info = new Geary.AccountInformation( - id, imap_information, smtp_information - ); - info.set_account_directories( - this.user_config_dir.get_child(id), - this.user_data_dir.get_child(id) + id, imap_info, smtp_info ); + info.set_account_directories(config_dir, data_dir); // This is the only required value at the moment? - string primary_email = key_file.get_value(Geary.Config.GROUP, Geary.Config.PRIMARY_EMAIL_KEY); - string real_name = Geary.Config.get_string_value(key_file, Geary.Config.GROUP, Geary.Config.REAL_NAME_KEY); + string primary_email = config.get_string(PRIMARY_EMAIL_KEY); + string real_name = config.get_string(REAL_NAME_KEY); - info.primary_mailbox = new Geary.RFC822.MailboxAddress(real_name, primary_email); - info.nickname = Geary.Config.get_string_value(key_file, Geary.Config.GROUP, Geary.Config.NICKNAME_KEY); + info.primary_mailbox = new Geary.RFC822.MailboxAddress( + real_name, primary_email + ); + info.nickname = config.get_string(NICKNAME_KEY); // Store alternate emails in a list of case-insensitive strings - Gee.List alt_email_list = Geary.Config.get_string_list_value(key_file, Geary.Config.GROUP, Geary.Config.ALTERNATE_EMAILS_KEY); + Gee.List alt_email_list = config.get_string_list( + ALTERNATE_EMAILS_KEY + ); if (alt_email_list.size != 0) { foreach (string alt_email in alt_email_list) { Geary.RFC822.MailboxAddresses mailboxes = new Geary.RFC822.MailboxAddresses.from_rfc822_string(alt_email); @@ -189,29 +221,35 @@ public class AccountManager : GLib.Object { } } - info.imap.load_credentials(key_file, primary_email); - info.smtp.load_credentials(key_file, primary_email); + info.imap.load_credentials(imap_config, primary_email); + info.smtp.load_credentials(smtp_config, primary_email); info.service_provider = Geary.ServiceProvider.from_string( - Geary.Config.get_string_value( - key_file, Geary.Config.GROUP, Geary.Config.SERVICE_PROVIDER_KEY, Geary.ServiceProvider.GMAIL.to_string())); - info.prefetch_period_days = Geary.Config.get_int_value( - key_file, Geary.Config.GROUP, Geary.Config.PREFETCH_PERIOD_DAYS_KEY, info.prefetch_period_days); - info.save_sent_mail = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, Geary.Config.SAVE_SENT_MAIL_KEY, info.save_sent_mail); - info.ordinal = Geary.Config.get_int_value( - key_file, Geary.Config.GROUP, Geary.Config.ORDINAL_KEY, info.ordinal); - info.use_email_signature = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, Geary.Config.USE_EMAIL_SIGNATURE_KEY, info.use_email_signature); - info.email_signature = Geary.Config.get_escaped_string( - key_file, Geary.Config.GROUP, Geary.Config.EMAIL_SIGNATURE_KEY, info.email_signature); + config.get_string(SERVICE_PROVIDER_KEY, + Geary.ServiceProvider.GMAIL.to_string()) + ); + info.prefetch_period_days = config.get_int( + PREFETCH_PERIOD_DAYS_KEY, info.prefetch_period_days + ); + info.save_sent_mail = config.get_bool( + SAVE_SENT_MAIL_KEY, info.save_sent_mail + ); + info.ordinal = config.get_int( + ORDINAL_KEY, info.ordinal + ); + info.use_email_signature = config.get_bool( + USE_EMAIL_SIGNATURE_KEY, info.use_email_signature + ); + info.email_signature = config.get_escaped_string( + EMAIL_SIGNATURE_KEY, info.email_signature + ); if (info.ordinal >= Geary.AccountInformation.next_ordinal) Geary.AccountInformation.next_ordinal = info.ordinal + 1; if (info.service_provider == Geary.ServiceProvider.OTHER) { - info.imap.load_settings(key_file); - info.smtp.load_settings(key_file); + info.imap.load_settings(imap_config); + info.smtp.load_settings(smtp_config); if (info.smtp.smtp_use_imap_credentials) { info.smtp.credentials.user = info.imap.credentials.user; @@ -220,99 +258,117 @@ public class AccountManager : GLib.Object { } info.drafts_folder_path = Geary.AccountInformation.build_folder_path( - Geary.Config.get_string_list_value(key_file, Geary.Config.GROUP, Geary.Config.DRAFTS_FOLDER_KEY)); + config.get_string_list(DRAFTS_FOLDER_KEY) + ); info.sent_mail_folder_path = Geary.AccountInformation.build_folder_path( - Geary.Config.get_string_list_value(key_file, Geary.Config.GROUP, Geary.Config.SENT_MAIL_FOLDER_KEY)); + config.get_string_list(SENT_MAIL_FOLDER_KEY) + ); info.spam_folder_path = Geary.AccountInformation.build_folder_path( - Geary.Config.get_string_list_value(key_file, Geary.Config.GROUP, Geary.Config.SPAM_FOLDER_KEY)); + config.get_string_list(SPAM_FOLDER_KEY) + ); info.trash_folder_path = Geary.AccountInformation.build_folder_path( - Geary.Config.get_string_list_value(key_file, Geary.Config.GROUP, Geary.Config.TRASH_FOLDER_KEY)); + config.get_string_list(TRASH_FOLDER_KEY) + ); info.archive_folder_path = Geary.AccountInformation.build_folder_path( - Geary.Config.get_string_list_value(key_file, Geary.Config.GROUP, Geary.Config.ARCHIVE_FOLDER_KEY)); + config.get_string_list(ARCHIVE_FOLDER_KEY) + ); - info.save_drafts = Geary.Config.get_bool_value(key_file, Geary.Config.GROUP, Geary.Config.SAVE_DRAFTS_KEY, true); + info.save_drafts = config.get_bool(SAVE_DRAFTS_KEY, true); return info; } - public async void store_to_file(Geary.AccountInformation info, - GLib.Cancellable? cancellable = null) { + public async void save_account(Geary.AccountInformation info, + GLib.Cancellable? cancellable = null) + throws GLib.Error { // Ensure only one async task is saving an info at once, since // at least the Engine can cause multiple saves to be called // in quick succession when updating special folder config. + int token = yield info.write_lock.claim_async(cancellable); + + GLib.Error? thrown = null; try { - int token = yield info.write_lock.claim_async(cancellable); - yield store_to_file_locked(info, cancellable); - info.write_lock.release(ref token); - } catch (Error err) { - debug("Error locking account info for saving: %s", err.message); + yield save_account_locked(info, cancellable); + } catch (GLib.Error err) { + thrown = err; + } + + info.write_lock.release(ref token); + + if (thrown != null) { + throw thrown; } } - private async void store_to_file_locked(Geary.AccountInformation info, - GLib.Cancellable? cancellable = null) { + private async void save_account_locked(Geary.AccountInformation info, + GLib.Cancellable? cancellable = null) + throws GLib.Error { File? file = info.settings_file; if (file == null) { - debug("Account information does not have a settings filed"); - return; + throw new AccountError.INVALID( + "Account information does not have a settings file" + ); } - if (!file.query_exists(cancellable)) { - try { - yield file.create_async(FileCreateFlags.REPLACE_DESTINATION); - } catch (Error err) { - debug("Error creating account info file: %s", err.message); - } + Geary.ConfigFile config_file = new Geary.ConfigFile(file); + + // Load the file first so we maintain old settings + try { + yield config_file.load(cancellable); + } catch (GLib.Error err) { + // Oh well, just create a new one when saving + debug("Could not load existing config file: %s", err.message); } - KeyFile key_file = new KeyFile(); - key_file.set_value(Geary.Config.GROUP, Geary.Config.CREDENTIALS_METHOD_KEY, info.imap.credentials_method.to_string()); - key_file.set_value(Geary.Config.GROUP, Geary.Config.REAL_NAME_KEY, info.primary_mailbox.name); - key_file.set_value(Geary.Config.GROUP, Geary.Config.PRIMARY_EMAIL_KEY, info.primary_mailbox.address); - key_file.set_value(Geary.Config.GROUP, Geary.Config.NICKNAME_KEY, info.nickname); - key_file.set_value(Geary.Config.GROUP, Geary.Config.SERVICE_PROVIDER_KEY, info.service_provider.to_string()); - key_file.set_integer(Geary.Config.GROUP, Geary.Config.ORDINAL_KEY, info.ordinal); - key_file.set_integer(Geary.Config.GROUP, Geary.Config.PREFETCH_PERIOD_DAYS_KEY, info.prefetch_period_days); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.SAVE_SENT_MAIL_KEY, info.save_sent_mail); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.USE_EMAIL_SIGNATURE_KEY, info.use_email_signature); - key_file.set_string(Geary.Config.GROUP, Geary.Config.EMAIL_SIGNATURE_KEY, info.email_signature); + Geary.ConfigFile.Group config = config_file.get_group(ACCOUNT_CONFIG_GROUP); + Geary.ConfigFile.Group imap_config = config_file.get_group(IMAP_CONFIG_GROUP); + Geary.ConfigFile.Group smtp_config = config_file.get_group(SMTP_CONFIG_GROUP); + + config.set_string( + CREDENTIALS_PROVIDER_KEY, CredentialsProvider.LIBSECRET.to_string() + ); + config.set_string( + CREDENTIALS_METHOD_KEY, info.imap.credentials_method.to_string() + ); + config.set_string(REAL_NAME_KEY, info.primary_mailbox.name); + config.set_string(PRIMARY_EMAIL_KEY, info.primary_mailbox.address); + config.set_string(NICKNAME_KEY, info.nickname); + config.set_string(SERVICE_PROVIDER_KEY, info.service_provider.to_string()); + config.set_int(ORDINAL_KEY, info.ordinal); + config.set_int(PREFETCH_PERIOD_DAYS_KEY, info.prefetch_period_days); + config.set_bool(SAVE_SENT_MAIL_KEY, info.save_sent_mail); + config.set_bool(USE_EMAIL_SIGNATURE_KEY, info.use_email_signature); + config.set_escaped_string(EMAIL_SIGNATURE_KEY, info.email_signature); if (info.alternate_mailboxes != null && info.alternate_mailboxes.size > 0) { string[] list = new string[info.alternate_mailboxes.size]; for (int ctr = 0; ctr < info.alternate_mailboxes.size; ctr++) list[ctr] = info.alternate_mailboxes[ctr].to_rfc822_string(); - key_file.set_string_list(Geary.Config.GROUP, Geary.Config.ALTERNATE_EMAILS_KEY, list); + config.set_string_list( + ALTERNATE_EMAILS_KEY, Geary.Collection.array_list_wrap(list) + ); } if (info.service_provider == Geary.ServiceProvider.OTHER) { - info.imap.save_settings(key_file); - info.smtp.save_settings(key_file); + info.imap.save_settings(imap_config); + info.smtp.save_settings(smtp_config); } - key_file.set_string_list(Geary.Config.GROUP, Geary.Config.DRAFTS_FOLDER_KEY, (info.drafts_folder_path != null - ? info.drafts_folder_path.as_list().to_array() : new string[] {})); - key_file.set_string_list(Geary.Config.GROUP, Geary.Config.SENT_MAIL_FOLDER_KEY, (info.sent_mail_folder_path != null - ? info.sent_mail_folder_path.as_list().to_array() : new string[] {})); - key_file.set_string_list(Geary.Config.GROUP,Geary. Config.SPAM_FOLDER_KEY, (info.spam_folder_path != null - ? info.spam_folder_path.as_list().to_array() : new string[] {})); - key_file.set_string_list(Geary.Config.GROUP, Geary.Config.TRASH_FOLDER_KEY, (info.trash_folder_path != null - ? info.trash_folder_path.as_list().to_array() : new string[] {})); - key_file.set_string_list(Geary.Config.GROUP, Geary.Config.ARCHIVE_FOLDER_KEY, (info.archive_folder_path != null - ? info.archive_folder_path.as_list().to_array() : new string[] {})); + Gee.LinkedList empty = new Gee.LinkedList(); + config.set_string_list(DRAFTS_FOLDER_KEY, (info.drafts_folder_path != null + ? info.drafts_folder_path.as_list() : empty)); + config.set_string_list(SENT_MAIL_FOLDER_KEY, (info.sent_mail_folder_path != null + ? info.sent_mail_folder_path.as_list() : empty)); + config.set_string_list(SPAM_FOLDER_KEY, (info.spam_folder_path != null + ? info.spam_folder_path.as_list() : empty)); + config.set_string_list(TRASH_FOLDER_KEY, (info.trash_folder_path != null + ? info.trash_folder_path.as_list() : empty)); + config.set_string_list(ARCHIVE_FOLDER_KEY, (info.archive_folder_path != null + ? info.archive_folder_path.as_list() : empty)); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.SAVE_DRAFTS_KEY, info.save_drafts); + config.set_bool(SAVE_DRAFTS_KEY, info.save_drafts); - string data = key_file.to_data(); - string new_etag; - - try { - yield file.replace_contents_async(data.data, null, false, FileCreateFlags.NONE, - cancellable, out new_etag); - - this.engine.add_account(info, true); - } catch (Error err) { - debug("Error writing to account info file: %s", err.message); - } + yield config_file.save(cancellable); } /** diff --git a/src/client/accounts/local-service-information.vala b/src/client/accounts/local-service-information.vala index 3581720f..5818ad5b 100644 --- a/src/client/accounts/local-service-information.vala +++ b/src/client/accounts/local-service-information.vala @@ -8,109 +8,65 @@ * from and to an account's configuration file. */ public class LocalServiceInformation : Geary.ServiceInformation { + + private const string HOST = "host"; + private const string PORT = "port"; + private const string REMEMBER_PASSWORD_KEY = "remember_password"; + private const string SMTP_NOAUTH = "smtp_noauth"; + private const string SMTP_USE_IMAP_CREDENTIALS = "smtp_use_imap_credentials"; + private const string SSL = "ssl"; + private const string STARTTLS = "starttls"; + private const string USERNAME_KEY = "username"; + + public LocalServiceInformation(Geary.Service service, + Geary.CredentialsMethod method, Geary.CredentialsMediator? mediator) { this.service = service; + this.credentials_method = method; this.mediator = mediator; - this.credentials_method = Geary.CredentialsMethod.PASSWORD; } - public override void load_settings(KeyFile key_file) throws Error { - string host_key = ""; - string port_key = ""; - string use_ssl_key = ""; - string use_starttls_key = ""; - uint16 default_port = 0; + public override void load_settings(Geary.ConfigFile.Group config) + throws GLib.Error { + this.host = config.get_string(HOST, this.host); + this.port = config.get_uint16(PORT, this.port); + this.use_ssl = config.get_bool(SSL, this.use_ssl); + this.use_starttls = config.get_bool(STARTTLS, this.use_starttls); - switch (service) { - case Geary.Service.IMAP: - host_key = Geary.Config.IMAP_HOST; - port_key = Geary.Config.IMAP_PORT; - use_ssl_key = Geary.Config.IMAP_SSL; - use_starttls_key = Geary.Config.IMAP_STARTTLS; - default_port = Geary.Imap.ClientConnection.DEFAULT_PORT_SSL; - break; - case Geary.Service.SMTP: - host_key = Geary.Config.SMTP_HOST; - port_key = Geary.Config.SMTP_PORT; - use_ssl_key = Geary.Config.SMTP_SSL; - use_starttls_key = Geary.Config.SMTP_STARTTLS; - default_port = Geary.Smtp.ClientConnection.DEFAULT_PORT_SSL; - this.smtp_noauth = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, Geary.Config.SMTP_NOAUTH, this.smtp_noauth); - if (smtp_noauth) - credentials = null; - this.smtp_use_imap_credentials = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, Geary.Config.SMTP_USE_IMAP_CREDENTIALS, this.smtp_use_imap_credentials); - break; + if (this.service == Geary.Service.SMTP) { + this.smtp_noauth = config.get_bool(SMTP_NOAUTH, this.smtp_noauth); + if (this.smtp_noauth) + this.credentials = null; + this.smtp_use_imap_credentials = config.get_bool( + SMTP_USE_IMAP_CREDENTIALS, + this.smtp_use_imap_credentials + ); } - this.host = Geary.Config.get_string_value( - key_file, Geary.Config.GROUP, host_key, this.host); - this.port = Geary.Config.get_uint16_value( - key_file, Geary.Config.GROUP, port_key, default_port); - this.use_ssl = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, use_ssl_key, this.use_ssl); - this.use_starttls = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, use_starttls_key, this.use_starttls); - - /* If the credentials provider and method keys are not in the config file, - * assume we have the libsecret credentials provider using plain password auth. - * Write these values back later when saving the configuration. */ - this.credentials_method = Geary.CredentialsMethod.from_string(Geary.Config.get_string_value( - key_file, Geary.Config.GROUP, Geary.Config.CREDENTIALS_METHOD_KEY, Geary.CredentialsMethod.PASSWORD.to_string())); } - public override void load_credentials(KeyFile key_file, string? email_address = null) throws Error { - string remember_password_key = ""; - string username_key = ""; - - switch (this.service) { - case Geary.Service.IMAP: - username_key = Geary.Config.IMAP_USERNAME_KEY; - remember_password_key = Geary.Config.IMAP_REMEMBER_PASSWORD_KEY; - break; - case Geary.Service.SMTP: - username_key = Geary.Config.SMTP_USERNAME_KEY; - remember_password_key = Geary.Config.SMTP_REMEMBER_PASSWORD_KEY; - break; - } - - this.credentials.user = Geary.Config.get_string_value( - key_file, Geary.Config.GROUP, username_key, email_address); - this.remember_password = Geary.Config.get_bool_value( - key_file, Geary.Config.GROUP, remember_password_key, this.remember_password); + public override void load_credentials(Geary.ConfigFile.Group config, + string? email_address = null) + throws GLib.Error { + this.credentials.user = config.get_string( + USERNAME_KEY, email_address + ); + this.remember_password = config.get_bool( + REMEMBER_PASSWORD_KEY, this.remember_password + ); } - public override void save_settings(KeyFile key_file) { - key_file.set_value( - Geary.Config.GROUP, - Geary.Config.CREDENTIALS_PROVIDER_KEY, - CredentialsProvider.LIBSECRET.to_string() - ); - key_file.set_value( - Geary.Config.GROUP, - Geary.Config.CREDENTIALS_METHOD_KEY, - this.credentials_method.to_string() - ); + public override void save_settings(Geary.ConfigFile.Group config) { + config.set_string(HOST, this.host); + config.set_int(PORT, this.port); + config.set_bool(SSL, this.use_ssl); + config.set_bool(STARTTLS, this.use_starttls); + config.set_string(USERNAME_KEY, this.credentials.user); - switch (this.service) { - case Geary.Service.IMAP: - key_file.set_value(Geary.Config.GROUP, Geary.Config.IMAP_HOST, this.host); - key_file.set_integer(Geary.Config.GROUP, Geary.Config.IMAP_PORT, this.port); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.IMAP_SSL, this.use_ssl); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.IMAP_STARTTLS, this.use_starttls); - key_file.set_string(Geary.Config.GROUP, Geary.Config.IMAP_USERNAME_KEY, this.credentials.user); - break; - case Geary.Service.SMTP: - key_file.set_value(Geary.Config.GROUP, Geary.Config.SMTP_HOST, this.host); - key_file.set_integer(Geary.Config.GROUP, Geary.Config.SMTP_PORT, this.port); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.SMTP_SSL, this.use_ssl); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.SMTP_STARTTLS, this.use_starttls); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.SMTP_USE_IMAP_CREDENTIALS, this.smtp_use_imap_credentials); - key_file.set_boolean(Geary.Config.GROUP, Geary.Config.SMTP_NOAUTH, this.smtp_noauth); - key_file.set_string(Geary.Config.GROUP, Geary.Config.SMTP_USERNAME_KEY, this.credentials.user); - break; + if (this.service == Geary.Service.SMTP) { + config.set_bool(SMTP_USE_IMAP_CREDENTIALS, this.smtp_use_imap_credentials); + config.set_bool(SMTP_NOAUTH, this.smtp_noauth); } } diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index 5b5dfe64..d881c26a 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -861,22 +861,27 @@ public class GearyController : Geary.BaseObject { real_account_information.copy_from(account_information); } - if (real_account_information.settings_file == null) { - try { + try { + if (real_account_information.settings_file == null) { yield this.account_manager.create_account_dirs( real_account_information ); - } catch (GLib.Error err) { - warning("Failed to create account directories"); } + yield this.account_manager.save_account(real_account_information); + yield do_update_stored_passwords_async( + Geary.ServiceFlag.IMAP | Geary.ServiceFlag.SMTP, + real_account_information + ); + debug("Successfully validated account information"); + } catch (GLib.Error err) { + report_problem( + new Geary.ProblemReport( + Geary.ProblemType.GENERIC_ERROR, err + ) + ); } - yield this.account_manager.store_to_file(real_account_information); - do_update_stored_passwords_async.begin(Geary.ServiceFlag.IMAP | Geary.ServiceFlag.SMTP, - real_account_information); - - debug("Successfully validated account information"); } - + return result; } @@ -1582,10 +1587,20 @@ public class GearyController : Geary.BaseObject { ); } - this.account_manager.store_to_file.begin( + this.account_manager.save_account.begin( info, null, (obj, res) => { - this.account_manager.store_to_file.end(res); + try { + this.account_manager.save_account.end(res); + } catch (GLib.Error err) { + report_problem( + new Geary.AccountProblemReport( + Geary.ProblemType.GENERIC_ERROR, + info, + err + ) + ); + } } ); } diff --git a/src/engine/api/geary-config.vala b/src/engine/api/geary-config.vala deleted file mode 100644 index 019db25a..00000000 --- a/src/engine/api/geary-config.vala +++ /dev/null @@ -1,98 +0,0 @@ -/* Copyright 2017 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -namespace Geary.Config { - public const string GROUP = "AccountInformation"; - public const string REAL_NAME_KEY = "real_name"; - public const string NICKNAME_KEY = "nickname"; - public const string PRIMARY_EMAIL_KEY = "primary_email"; - public const string ALTERNATE_EMAILS_KEY = "alternate_emails"; - public const string SERVICE_PROVIDER_KEY = "service_provider"; - public const string ORDINAL_KEY = "ordinal"; - public const string PREFETCH_PERIOD_DAYS_KEY = "prefetch_period_days"; - public const string IMAP_USERNAME_KEY = "imap_username"; - public const string IMAP_REMEMBER_PASSWORD_KEY = "imap_remember_password"; - public const string SMTP_USERNAME_KEY = "smtp_username"; - public const string SMTP_REMEMBER_PASSWORD_KEY = "smtp_remember_password"; - public const string IMAP_HOST = "imap_host"; - public const string IMAP_PORT = "imap_port"; - public const string IMAP_SSL = "imap_ssl"; - public const string IMAP_STARTTLS = "imap_starttls"; - public const string SMTP_HOST = "smtp_host"; - public const string SMTP_PORT = "smtp_port"; - public const string SMTP_SSL = "smtp_ssl"; - public const string SMTP_STARTTLS = "smtp_starttls"; - public const string SMTP_USE_IMAP_CREDENTIALS = "smtp_use_imap_credentials"; - public const string SMTP_NOAUTH = "smtp_noauth"; - public const string SAVE_SENT_MAIL_KEY = "save_sent_mail"; - public const string DRAFTS_FOLDER_KEY = "drafts_folder"; - public const string SENT_MAIL_FOLDER_KEY = "sent_mail_folder"; - public const string SPAM_FOLDER_KEY = "spam_folder"; - public const string TRASH_FOLDER_KEY = "trash_folder"; - public const string ARCHIVE_FOLDER_KEY = "archive_folder"; - public const string SAVE_DRAFTS_KEY = "save_drafts"; - public const string USE_EMAIL_SIGNATURE_KEY = "use_email_signature"; - public const string EMAIL_SIGNATURE_KEY = "email_signature"; - public const string CREDENTIALS_PROVIDER_KEY = "credentials_provider"; - public const string CREDENTIALS_METHOD_KEY = "credentials_method"; - public const string SETTINGS_FILENAME = "geary.ini"; - - public static string get_string_value(KeyFile key_file, string group, string key, string def = "") { - try { - return key_file.get_string(group, key); - } catch(KeyFileError err) { - // Ignore. - } - - return def; - } - - public static string get_escaped_string(KeyFile key_file, string group, string key, string def = "") { - try { - return key_file.get_value(group, key); - } catch (KeyFileError err) { - // ignore - } - - return def; - } - - public static Gee.List get_string_list_value(KeyFile key_file, string group, string key) { - try { - string[] list = key_file.get_string_list(group, key); - if (list.length > 0) - return Geary.Collection.array_list_wrap(list); - } catch(KeyFileError err) { - // Ignore. - } - - return new Gee.ArrayList(); - } - - public static bool get_bool_value(KeyFile key_file, string group, string key, bool def = false) { - try { - return key_file.get_boolean(group, key); - } catch(KeyFileError err) { - // Ignore. - } - - return def; - } - - public static int get_int_value(KeyFile key_file, string group, string key, int def = 0) { - try { - return key_file.get_integer(group, key); - } catch(KeyFileError err) { - // Ignore. - } - - return def; - } - - public static uint16 get_uint16_value(KeyFile key_file, string group, string key, uint16 def = 0) { - return (uint16) get_int_value(key_file, group, key); - } -} diff --git a/src/engine/api/geary-service-information.vala b/src/engine/api/geary-service-information.vala index 32c4e11d..a4f273e6 100644 --- a/src/engine/api/geary-service-information.vala +++ b/src/engine/api/geary-service-information.vala @@ -96,17 +96,17 @@ public abstract class Geary.ServiceInformation : GLib.Object { * * This method depends on the concrete implementation used. */ - public abstract void load_settings(KeyFile key_file) throws Error; + public abstract void load_settings(ConfigFile.Group config) throws Error; /** * Loads the credentials pertaining to this class's instance. * * This method depends on the concrete implementation used. */ - public abstract void load_credentials(KeyFile key_file, string? email_address = null) throws Error; + public abstract void load_credentials(ConfigFile.Group config, string? email_address = null) throws Error; /** Saves settings pertaining to this class's instance to a key file. */ - public abstract void save_settings(KeyFile key_file); + public abstract void save_settings(ConfigFile.Group config); public void copy_from(Geary.ServiceInformation from) { this.host = from.host; diff --git a/src/engine/meson.build b/src/engine/meson.build index 265a24d7..9787001a 100644 --- a/src/engine/meson.build +++ b/src/engine/meson.build @@ -8,7 +8,6 @@ geary_engine_vala_sources = files( 'api/geary-attachment.vala', 'api/geary-base-object.vala', 'api/geary-composed-email.vala', - 'api/geary-config.vala', 'api/geary-contact.vala', 'api/geary-contact-flags.vala', 'api/geary-contact-importance.vala', @@ -298,6 +297,7 @@ geary_engine_vala_sources = files( 'util/util-ascii.vala', 'util/util-collection.vala', + 'util/util-config-file.vala', 'util/util-connectivity-manager.vala', 'util/util-files.vala', 'util/util-generic-capabilities.vala', diff --git a/src/engine/util/util-config-file.vala b/src/engine/util/util-config-file.vala new file mode 100644 index 00000000..00972c34 --- /dev/null +++ b/src/engine/util/util-config-file.vala @@ -0,0 +1,216 @@ +/* + * Copyright 2018 Michael James Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +/** + * A simple ini-file-like configuration file. + */ +public class Geary.ConfigFile { + + + /** A set of configuration items under a "[Name]" heading. */ + public class Group { + + + private struct GroupLookup { + string group; + string prefix; + + public GroupLookup(string group, string prefix) { + this.group = group; + this.prefix = prefix; + } + } + + + /** The name of this group, as specified by a [Name] heading. */ + public string name { get; private set; } + + private GLib.KeyFile backing; + private GroupLookup[] lookups; + + + internal Group(string name, GLib.KeyFile backing) { + this.name = name; + this.backing = backing; + + this.lookups = { GroupLookup(name, "") }; + } + + + /** + * Sets a fallback lookup for missing keys in the group. + * + * This provides a fallback for looking up a legacy key. If + * set, when performing a lookup for `key` and no such key in + * the group is found, a lookup in the alternative specified + * group for `prefix` + `key` will be performed and returned + * if found. + */ + public void set_fallback(string group, string prefix) { + this.lookups = { this.lookups[0], GroupLookup(group, prefix) }; + } + + + public string get_string(string key, string def = "") { + string ret = def; + foreach (GroupLookup lookup in this.lookups) { + try { + ret = this.backing.get_string( + lookup.group, lookup.prefix + key + ); + break; + } catch (GLib.KeyFileError err) { + // continue + } + } + return ret; + } + + public void set_string(string key, string value) { + this.backing.set_string(this.name, key, value); + } + + public string get_escaped_string(string key, string def = "") { + string ret = def; + foreach (GroupLookup lookup in this.lookups) { + try { + ret = this.backing.get_value( + lookup.group, lookup.prefix + key + ); + break; + } catch (GLib.KeyFileError err) { + // continue + } + } + return ret; + } + + public void set_escaped_string(string key, string value) { + this.backing.set_value(this.name, key, value); + } + + public Gee.List get_string_list(string key) { + try { + string[] list = this.backing.get_string_list(this.name, key); + if (list.length > 0) + return Geary.Collection.array_list_wrap(list); + } catch (GLib.KeyFileError err) { + // Oh well + } + return new Gee.ArrayList(); + } + + public void set_string_list(string key, Gee.List value) { + this.backing.set_string_list(this.name, key, value.to_array()); + } + + public bool get_bool(string key, bool def = false) { + bool ret = def; + foreach (GroupLookup lookup in this.lookups) { + try { + ret = this.backing.get_boolean( + lookup.group, lookup.prefix + key + ); + break; + } catch (GLib.KeyFileError err) { + // continue + } + } + return ret; + } + + public void set_bool(string key, bool value) { + this.backing.set_boolean(this.name, key, value); + } + + public int get_int(string key, int def = 0) { + int ret = def; + foreach (GroupLookup lookup in this.lookups) { + try { + ret = this.backing.get_integer( + lookup.group, lookup.prefix + key + ); + break; + } catch (GLib.KeyFileError err) { + // continue + } + } + return ret; + } + + public void set_int(string key, int value) { + this.backing.set_integer(this.name, key, value); + } + + public uint16 get_uint16(string key, uint16 def = 0) { + return (uint16) get_int(key, (int) def); + } + + public void set_uint16(string key, uint16 value) { + this.backing.set_integer(this.name, key, (int) value); + } + + } + + + private GLib.File config_file; + private GLib.KeyFile backing = new KeyFile(); + + + /** + * Constructs a config file using the specified on-disk file. + */ + public ConfigFile(GLib.File config_file) { + this.config_file = config_file; + } + + /** + * Returns the config group under the given named heading. + */ + public Group get_group(string name) { + return new Group(name, this.backing); + } + + /** + * Loads config data from the underlying config file. + */ + public async void load(GLib.Cancellable? cancellable = null) + throws GLib.Error { + GLib.Error? thrown = null; + yield Nonblocking.Concurrent.global.schedule_async(() => { + try { + this.backing.load_from_file( + this.config_file.get_path(), KeyFileFlags.NONE + ); + } catch (GLib.Error err) { + thrown = err; + } + }, cancellable); + if (thrown != null) { + throw thrown; + } + } + + /** + * Saves config data to the underlying config file. + */ + public async void save(GLib.Cancellable? cancellable = null) + throws GLib.Error { + GLib.Error? thrown = null; + yield Nonblocking.Concurrent.global.schedule_async(() => { + try { + this.backing.save_to_file(this.config_file.get_path()); + } catch (GLib.Error err) { + thrown = err; + } + }, cancellable); + if (thrown != null) { + throw thrown; + } + } + +} diff --git a/test/engine/api/geary-service-information-mock.vala b/test/engine/api/geary-service-information-mock.vala index 3a04c930..b1592589 100644 --- a/test/engine/api/geary-service-information-mock.vala +++ b/test/engine/api/geary-service-information-mock.vala @@ -13,20 +13,20 @@ public class Geary.MockServiceInformation : ServiceInformation, MockObject { } - public override void load_settings(KeyFile key_file) + public override void load_settings(Geary.ConfigFile.Group config) throws Error { - void_call("load_settings", { box_arg(key_file) }); + void_call("load_settings", { box_arg(config) }); } - public override void load_credentials(KeyFile key_file, + public override void load_credentials(Geary.ConfigFile.Group config, string? email_address = null) throws Error { - void_call("load_credentials", { box_arg(key_file), box_arg(email_address) }); + void_call("load_credentials", { box_arg(config), box_arg(email_address) }); } - public override void save_settings(KeyFile key_file) { + public override void save_settings(Geary.ConfigFile.Group config) { try { - void_call("save_settings", { box_arg(key_file) }); + void_call("save_settings", { box_arg(config) }); } catch (Error err) { assert_not_reached(); } diff --git a/test/engine/util-config-file-test.vala b/test/engine/util-config-file-test.vala new file mode 100644 index 00000000..85298fb5 --- /dev/null +++ b/test/engine/util-config-file-test.vala @@ -0,0 +1,94 @@ +/* + * Copyright 2018 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +class Geary.ConfigFileTest : TestCase { + + + private const string TEST_KEY = "test-key"; + private const string TEST_KEY_MISSING = "test-key-missing"; + + private ConfigFile? test_config = null; + private ConfigFile.Group? test_group = null; + + + public ConfigFileTest() { + base("Geary.ConfigFileTest"); + add_test("test_string", test_string); + add_test("test_string_fallback", test_string_fallback); + add_test("test_escaped_string", test_escaped_string); + add_test("test_string_list", test_string_list); + add_test("test_string_list", test_string_list); + add_test("test_bool", test_bool); + add_test("test_int", test_int); + add_test("test_uint16", test_uint16); + } + + public override void set_up() throws GLib.Error { + this.test_config = new ConfigFile(GLib.File.new_for_path("/tmp/config.ini")); + this.test_group = this.test_config.get_group("Test"); + } + + public override void tear_down() throws GLib.Error { + this.test_group = null; + this.test_config = null; + } + + public void test_string() throws Error { + this.test_group.set_string(TEST_KEY, "a string"); + assert_string("a string", this.test_group.get_string(TEST_KEY)); + assert_string("default", this.test_group.get_string(TEST_KEY_MISSING, "default")); + } + + public void test_string_fallback() throws Error { + ConfigFile.Group fallback = this.test_config.get_group("fallback"); + fallback.set_string("fallback-test-key", "a string"); + + this.test_group.set_fallback("fallback", "fallback-"); + assert_string("a string", this.test_group.get_string(TEST_KEY)); + } + + public void test_escaped_string() throws Error { + this.test_group.set_escaped_string(TEST_KEY, "a = string"); + assert_string("a = string", this.test_group.get_escaped_string(TEST_KEY)); + assert_string("=default", this.test_group.get_escaped_string(TEST_KEY_MISSING, "=default")); + } + + public void test_string_list() throws Error { + this.test_group.set_string_list( + TEST_KEY, new Gee.ArrayList.wrap({ "a", "b"}) + ); + + Gee.List saved = this.test_group.get_string_list(TEST_KEY); + assert_int(2, saved.size, "Saved string list"); + assert_string("a", saved[0]); + assert_string("b", saved[1]); + + Gee.List def = this.test_group.get_string_list(TEST_KEY_MISSING); + assert_int(0, def.size, "Default string list"); + } + + public void test_bool() throws Error { + this.test_group.set_bool(TEST_KEY, true); + assert_true(this.test_group.get_bool(TEST_KEY)); + assert_true(this.test_group.get_bool(TEST_KEY_MISSING, true)); + assert_false(this.test_group.get_bool(TEST_KEY_MISSING, false)); + } + + public void test_int() throws Error { + this.test_group.set_int(TEST_KEY, 42); + assert_int(42, this.test_group.get_int(TEST_KEY)); + assert_int(42, this.test_group.get_int(TEST_KEY_MISSING, 42)); + } + + public void test_uint16() throws Error { + this.test_group.set_uint16(TEST_KEY, 42); + assert_int(42, this.test_group.get_uint16(TEST_KEY)); + assert_int(42, this.test_group.get_uint16(TEST_KEY_MISSING, 42)); + } + + +} diff --git a/test/meson.build b/test/meson.build index 869ae11e..bd2fbd92 100644 --- a/test/meson.build +++ b/test/meson.build @@ -41,6 +41,7 @@ geary_test_engine_sources = [ 'engine/rfc822-part-test.vala', 'engine/rfc822-utils-test.vala', 'engine/util-ascii-test.vala', + 'engine/util-config-file-test.vala', 'engine/util-html-test.vala', 'engine/util-idle-manager-test.vala', 'engine/util-inet-test.vala', diff --git a/test/test-engine.vala b/test/test-engine.vala index 82e1664e..e5887fa9 100644 --- a/test/test-engine.vala +++ b/test/test-engine.vala @@ -31,6 +31,7 @@ int main(string[] args) { // Depends on ConversationTest and ConversationSetTest passing engine.add_suite(new Geary.App.ConversationMonitorTest().get_suite()); engine.add_suite(new Geary.Ascii.Test().get_suite()); + engine.add_suite(new Geary.ConfigFileTest().get_suite()); engine.add_suite(new Geary.Db.DatabaseTest().get_suite()); engine.add_suite(new Geary.Db.VersionedDatabaseTest().get_suite()); engine.add_suite(new Geary.HTML.UtilTest().get_suite());