From 95ebdd65d2bf7132d87d80641e978799973fe617 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 27 Dec 2018 10:20:58 +1030 Subject: [PATCH] Tidy up ClientService API a bit Require endpoint to always be present so it is never null, and hence passed in through account creation. Rename props to be a bit more descriptive. --- src/client/application/geary-controller.vala | 8 ++-- src/engine/api/geary-client-service.vala | 39 +++++++++--------- src/engine/api/geary-engine.vala | 34 ++++++++------- .../gmail/imap-engine-gmail-account.vala | 7 +++- .../imap-engine-generic-account.vala | 18 +++++--- .../other/imap-engine-other-account.vala | 7 +++- .../outlook/imap-engine-outlook-account.vala | 7 +++- .../yahoo/imap-engine-yahoo-account.vala | 7 +++- src/engine/imap/api/imap-client-service.vala | 41 ++++++++++--------- src/engine/smtp/smtp-client-service.vala | 19 +++++---- test/engine/api/geary-account-mock.vala | 17 ++++++-- 11 files changed, 120 insertions(+), 84 deletions(-) diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index b3bae268..dda59ec8 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -831,8 +831,8 @@ public class GearyController : Geary.BaseObject { // they agreed to both, try again Geary.Account account = this.accounts.get(config).account; if (imap_prompted && smtp_prompted - && account.incoming.endpoint.is_trusted_or_never_connected - && account.outgoing.endpoint.is_trusted_or_never_connected) { + && account.incoming.remote.is_trusted_or_never_connected + && account.outgoing.remote.is_trusted_or_never_connected) { retry_required = true; } else if (validation_result == Geary.Engine.ValidationResult.OK) { retry_required = true; @@ -2893,10 +2893,10 @@ public class GearyController : Geary.BaseObject { Geary.Endpoint? endpoint = null; switch (service.protocol) { case Geary.Protocol.IMAP: - endpoint = account.incoming.endpoint; + endpoint = account.incoming.remote; break; case Geary.Protocol.SMTP: - endpoint = account.outgoing.endpoint; + endpoint = account.outgoing.remote; break; } return endpoint; diff --git a/src/engine/api/geary-client-service.vala b/src/engine/api/geary-client-service.vala index 0582188a..5033fee5 100644 --- a/src/engine/api/geary-client-service.vala +++ b/src/engine/api/geary-client-service.vala @@ -18,28 +18,30 @@ public abstract class Geary.ClientService : BaseObject { /** - * The configuration for the account the service belongs to. + * The service's account. */ public AccountInformation account { get; private set; } /** * The configuration for the service. */ - public ServiceInformation service { get; private set; } + public ServiceInformation configuration { get; private set; } /** * The network endpoint the service will connect to. */ - public Endpoint? endpoint { get; private set; default = null; } + public Endpoint remote { get; private set; } /** Determines if this manager has been started. */ public bool is_running { get; protected set; default = false; } protected ClientService(AccountInformation account, - ServiceInformation service) { + ServiceInformation configuration, + Endpoint remote) { this.account = account; - this.service = service; + this.configuration = configuration; + this.remote = remote; } /** @@ -52,23 +54,20 @@ public abstract class Geary.ClientService : BaseObject { public async void set_endpoint_restart(Endpoint endpoint, GLib.Cancellable? cancellable = null) throws GLib.Error { - if ((this.endpoint == null && endpoint != null) || - (this.endpoint != null && this.endpoint.equal_to(endpoint))) { - if (this.endpoint != null) { - this.endpoint.untrusted_host.disconnect(on_untrusted_host); - } + if (this.remote != null) { + this.remote.untrusted_host.disconnect(on_untrusted_host); + } - bool do_restart = this.is_running; - if (do_restart) { - yield stop(cancellable); - } + bool do_restart = this.is_running; + if (do_restart) { + yield stop(cancellable); + } - this.endpoint = endpoint; - this.endpoint.untrusted_host.connect(on_untrusted_host); + this.remote = remote; + this.remote.untrusted_host.connect(on_untrusted_host); - if (do_restart) { - yield start(cancellable); - } + if (do_restart) { + yield start(cancellable); } } @@ -92,7 +91,7 @@ public abstract class Geary.ClientService : BaseObject { private void on_untrusted_host(TlsNegotiationMethod method, GLib.TlsConnection cx) { - this.account.untrusted_host(this.service, method, cx); + this.account.untrusted_host(this.configuration, method, cx); } } diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala index 08998c39..b5b9d72b 100644 --- a/src/engine/api/geary-engine.vala +++ b/src/engine/api/geary-engine.vala @@ -352,38 +352,44 @@ public class Geary.Engine : BaseObject { if (account_instances.has_key(config.id)) return account_instances.get(config.id); + ImapDB.Account local = new ImapDB.Account(config); + 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); + account = new ImapEngine.GmailAccount( + config, local, incoming_remote, outgoing_remote + ); break; case ServiceProvider.YAHOO: - account = new ImapEngine.YahooAccount(config); + account = new ImapEngine.YahooAccount( + config, local, incoming_remote, outgoing_remote + ); break; case ServiceProvider.OUTLOOK: - account = new ImapEngine.OutlookAccount(config); + account = new ImapEngine.OutlookAccount( + config, local, incoming_remote, outgoing_remote + ); break; case ServiceProvider.OTHER: - account = new ImapEngine.OtherAccount(config); + account = new ImapEngine.OtherAccount( + config, local, incoming_remote, outgoing_remote + ); break; default: assert_not_reached(); } - Endpoint incoming = get_shared_endpoint( - config.service_provider, config.incoming - ); - account.set_endpoint(account.incoming, incoming); - - Endpoint outgoing = get_shared_endpoint( - config.service_provider, config.outgoing - ); - account.set_endpoint(account.outgoing, outgoing); - account_instances.set(config.id, account); return account; } diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala index 2e9567be..0857352c 100644 --- a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala +++ b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala @@ -32,8 +32,11 @@ private class Geary.ImapEngine.GmailAccount : Geary.ImapEngine.GenericAccount { } - public GmailAccount(Geary.AccountInformation config) { - base(config); + public GmailAccount(Geary.AccountInformation config, + ImapDB.Account local, + Endpoint incoming_remote, + Endpoint outgoing_remote) { + base(config, local, incoming_remote, outgoing_remote); } protected override Geary.SpecialFolderType[] get_supported_special_folders() { diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 041ccb11..785374fe 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -62,19 +62,27 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { new Gee.HashMap>(); - public GenericAccount(AccountInformation config) { + public GenericAccount(AccountInformation config, + ImapDB.Account local, + Endpoint incoming_remote, + Endpoint outgoing_remote) { base(config); - this.local = new ImapDB.Account(config); + this.local = local; this.local.contacts_loaded.connect(() => { contacts_loaded(); }); - this.imap = new Imap.ClientService(config, config.incoming); + this.imap = new Imap.ClientService( + config, config.incoming, incoming_remote + ); this.imap.min_pool_size = IMAP_MIN_POOL_SIZE; this.imap.ready.connect(on_pool_session_ready); this.imap.connection_failed.connect(on_pool_connection_failed); this.imap.login_failed.connect(on_pool_login_failed); this.smtp = new Smtp.ClientService( - config, config.outgoing, new Outbox.Folder(this, this.local) + config, + config.outgoing, + outgoing_remote, + new Outbox.Folder(this, this.local) ); this.smtp.email_sent.connect(on_email_sent); this.smtp.report_problem.connect(notify_report_problem); @@ -488,7 +496,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { // that's supposed to be globally unique... Geary.RFC822.Message rfc822 = new Geary.RFC822.Message.from_composed_email( composed, GMime.utils_generate_message_id( - this.smtp.endpoint.remote_address.hostname + this.smtp.remote.remote_address.hostname )); yield this.smtp.queue_email(rfc822, cancellable); diff --git a/src/engine/imap-engine/other/imap-engine-other-account.vala b/src/engine/imap-engine/other/imap-engine-other-account.vala index 85ff844a..e3eb94a6 100644 --- a/src/engine/imap-engine/other/imap-engine-other-account.vala +++ b/src/engine/imap-engine/other/imap-engine-other-account.vala @@ -7,8 +7,11 @@ private class Geary.ImapEngine.OtherAccount : Geary.ImapEngine.GenericAccount { - public OtherAccount(AccountInformation config) { - base(config); + public OtherAccount(AccountInformation config, + ImapDB.Account local, + Endpoint incoming_remote, + Endpoint outgoing_remote) { + base(config, local, incoming_remote, outgoing_remote); } protected override MinimalFolder new_folder(ImapDB.Folder local_folder) { diff --git a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala index 57c1c948..2c09c3e5 100644 --- a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala +++ b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala @@ -24,8 +24,11 @@ private class Geary.ImapEngine.OutlookAccount : Geary.ImapEngine.GenericAccount } - public OutlookAccount(AccountInformation config) { - base(config); + public OutlookAccount(AccountInformation config, + ImapDB.Account local, + Endpoint incoming_remote, + Endpoint outgoing_remote) { + base(config, local, incoming_remote, outgoing_remote); } protected override MinimalFolder new_folder(ImapDB.Folder local_folder) { diff --git a/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala b/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala index bbdc543d..b64c5afb 100644 --- a/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala +++ b/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala @@ -27,8 +27,11 @@ private class Geary.ImapEngine.YahooAccount : Geary.ImapEngine.GenericAccount { } - public YahooAccount(AccountInformation config) { - base(config); + public YahooAccount(AccountInformation config, + ImapDB.Account local, + Endpoint incoming_remote, + Endpoint outgoing_remote) { + base(config, local, incoming_remote, outgoing_remote); if (special_map == null) { special_map = new Gee.HashMap(); diff --git a/src/engine/imap/api/imap-client-service.vala b/src/engine/imap/api/imap-client-service.vala index 62fc66d3..e37b55eb 100644 --- a/src/engine/imap/api/imap-client-service.vala +++ b/src/engine/imap/api/imap-client-service.vala @@ -125,8 +125,9 @@ internal class Geary.Imap.ClientService : Geary.ClientService { public ClientService(AccountInformation account, - ServiceInformation service) { - base(account, service); + ServiceInformation service, + Endpoint remote) { + base(account, service, remote); this.pool_start = new TimeoutManager.seconds( POOL_START_TIMEOUT_SEC, @@ -154,20 +155,20 @@ internal class Geary.Imap.ClientService : Geary.ClientService { this.authentication_failed = false; this.pool_cancellable = new Cancellable(); - this.endpoint.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].connect( + this.remote.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].connect( on_imap_trust_untrusted_host ); - this.endpoint.untrusted_host.connect(on_imap_untrusted_host); - this.endpoint.connectivity.notify["is-reachable"].connect( + this.remote.untrusted_host.connect(on_imap_untrusted_host); + this.remote.connectivity.notify["is-reachable"].connect( on_connectivity_change ); - this.endpoint.connectivity.address_error_reported.connect( + this.remote.connectivity.address_error_reported.connect( on_connectivity_error ); - if (this.endpoint.connectivity.is_reachable.is_certain()) { + if (this.remote.connectivity.is_reachable.is_certain()) { this.check_pool.begin(); } else { - this.endpoint.connectivity.check_reachable.begin(); + this.remote.connectivity.check_reachable.begin(); } } @@ -183,14 +184,14 @@ internal class Geary.Imap.ClientService : Geary.ClientService { this.is_running = false; this.pool_cancellable.cancel(); - this.endpoint.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].disconnect( + this.remote.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].disconnect( on_imap_trust_untrusted_host ); - this.endpoint.untrusted_host.disconnect(on_imap_untrusted_host); - this.endpoint.connectivity.notify["is-reachable"].disconnect( + this.remote.untrusted_host.disconnect(on_imap_untrusted_host); + this.remote.connectivity.notify["is-reachable"].disconnect( on_connectivity_change ); - this.endpoint.connectivity.address_error_reported.disconnect( + this.remote.connectivity.address_error_reported.disconnect( on_connectivity_error ); @@ -238,10 +239,10 @@ internal class Geary.Imap.ClientService : Geary.ClientService { throw new ImapError.UNAUTHENTICATED("Invalid ClientSessionManager credentials"); if (this.untrusted_host) - throw new ImapError.UNAVAILABLE("Untrusted host %s", endpoint.to_string()); + throw new ImapError.UNAVAILABLE("Untrusted host %s", remote.to_string()); - if (!this.endpoint.connectivity.is_reachable.is_certain()) - throw new ImapError.UNAVAILABLE("Host at %s is unreachable", endpoint.to_string()); + if (!this.remote.connectivity.is_reachable.is_certain()) + throw new ImapError.UNAVAILABLE("Host at %s is unreachable", remote.to_string()); ClientSession? claimed = null; while (claimed == null) { @@ -330,7 +331,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { if (this.is_running && !this.authentication_failed && !this.untrusted_host && - this.endpoint.connectivity.is_reachable.is_certain()) { + this.remote.connectivity.is_reachable.is_certain()) { int needed = this.min_pool_size - this.all_sessions.size; if (needed <= 0 && is_claiming) { @@ -424,7 +425,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { private async ClientSession create_new_authorized_session(Cancellable? cancellable) throws Error { debug("[%s] Opening new session", this.account.id); - ClientSession new_session = new ClientSession(endpoint); + ClientSession new_session = new ClientSession(remote); // Listen for auth failures early so the client is notified if // there is an error, even though we won't want to keep the @@ -442,7 +443,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { try { yield new_session.initiate_session_async( - this.service.credentials, cancellable + this.configuration.credentials, cancellable ); } catch (Error err) { if (!(err is IOError.CANCELLED)) { @@ -571,7 +572,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { private void on_imap_trust_untrusted_host() { // fired when the trust_untrusted_host property changes, indicating if the user has agreed // to ignore the trust problems and continue connecting - if (untrusted_host && endpoint.trust_untrusted_host == Trillian.TRUE) { + if (untrusted_host && remote.trust_untrusted_host == Trillian.TRUE) { untrusted_host = false; if (this.is_running) { @@ -581,7 +582,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { } private void on_connectivity_change() { - bool is_reachable = this.endpoint.connectivity.is_reachable.is_certain(); + bool is_reachable = this.remote.connectivity.is_reachable.is_certain(); if (is_reachable) { this.pool_start.start(); this.pool_stop.reset(); diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala index 42b3f2a6..d7eb1f17 100644 --- a/src/engine/smtp/smtp-client-service.vala +++ b/src/engine/smtp/smtp-client-service.vala @@ -52,8 +52,9 @@ internal class Geary.Smtp.ClientService : Geary.ClientService { public ClientService(AccountInformation account, ServiceInformation service, + Endpoint remote, Outbox.Folder outbox) { - base(account, service); + base(account, service, remote); this.outbox = outbox; } @@ -65,13 +66,13 @@ internal class Geary.Smtp.ClientService : Geary.ClientService { this.is_running = true; yield this.outbox.open_async(Folder.OpenFlags.NONE, cancellable); yield this.fill_outbox_queue(cancellable); - this.endpoint.connectivity.notify["is-reachable"].connect( + this.remote.connectivity.notify["is-reachable"].connect( on_reachable_changed ); - this.endpoint.connectivity.address_error_reported.connect( + this.remote.connectivity.address_error_reported.connect( on_connectivity_error ); - this.endpoint.connectivity.check_reachable.begin(); + this.remote.connectivity.check_reachable.begin(); } /** @@ -79,10 +80,10 @@ internal class Geary.Smtp.ClientService : Geary.ClientService { */ public override async void stop(GLib.Cancellable? cancellable = null) throws GLib.Error { - this.endpoint.connectivity.notify["is-reachable"].disconnect( + this.remote.connectivity.notify["is-reachable"].disconnect( on_reachable_changed ); - this.endpoint.connectivity.address_error_reported.disconnect( + this.remote.connectivity.address_error_reported.disconnect( on_connectivity_error ); this.stop_postie(); @@ -311,7 +312,7 @@ internal class Geary.Smtp.ClientService : Geary.ClientService { private async void send_email(Geary.RFC822.Message rfc822, Cancellable? cancellable) throws Error { - Smtp.ClientSession smtp = new Geary.Smtp.ClientSession(this.endpoint); + Smtp.ClientSession smtp = new Geary.Smtp.ClientSession(this.remote); sending_monitor.notify_start(); @@ -408,12 +409,12 @@ internal class Geary.Smtp.ClientService : Geary.ClientService { private void notify_report_problem(ProblemType problem, Error? err) { report_problem( - new ServiceProblemReport(problem, this.account, this.service, err) + new ServiceProblemReport(problem, this.account, this.configuration, err) ); } private void on_reachable_changed() { - if (this.endpoint.connectivity.is_reachable.is_certain()) { + if (this.remote.connectivity.is_reachable.is_certain()) { start_postie.begin(); } else { stop_postie(); diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala index b1efca93..4696d2eb 100644 --- a/test/engine/api/geary-account-mock.vala +++ b/test/engine/api/geary-account-mock.vala @@ -34,8 +34,9 @@ public class Geary.MockAccount : Account, MockObject { public class MockClientService : ClientService { public MockClientService(AccountInformation account, - ServiceInformation service) { - base(account, service); + ServiceInformation configuration, + Endpoint remote) { + base(account, configuration, remote); } public override async void start(GLib.Cancellable? cancellable = null) @@ -70,8 +71,16 @@ public class Geary.MockAccount : Account, MockObject { public MockAccount(AccountInformation config) { base(config); - this._incoming = new MockClientService(config, config.incoming); - this._outgoing = new MockClientService(config, config.outgoing); + this._incoming = new MockClientService( + config, + config.incoming, + new Endpoint(config.incoming.host, config.incoming.port, 0, 0) + ); + this._outgoing = new MockClientService( + config, + config.outgoing, + new Endpoint(config.outgoing.host, config.outgoing.port, 0, 0) + ); } public override async void open_async(Cancellable? cancellable = null) throws Error {