diff --git a/src/engine/imap/api/imap-client-service.vala b/src/engine/imap/api/imap-client-service.vala index adbd9b4d..1697ec73 100644 --- a/src/engine/imap/api/imap-client-service.vala +++ b/src/engine/imap/api/imap-client-service.vala @@ -227,7 +227,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { } else if (yield check_session(session, false)) { bool free = true; MailboxSpecifier? mailbox = null; - ClientSession.ProtocolState proto = session.get_protocol_state(out mailbox); + ClientSession.ProtocolState proto = session.get_protocol_state(); // If the session has a mailbox selected, close it before // adding it back to the pool if (proto == ClientSession.ProtocolState.SELECTED || @@ -242,7 +242,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { } // Double check the session after closing it - switch (session.get_protocol_state(null)) { + switch (session.get_protocol_state()) { case AUTHORIZED: // This is the desired state, so all good break; @@ -381,7 +381,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService { /** Determines if a session is valid, disposing of it if not. */ private async bool check_session(ClientSession target, bool claiming) { bool valid = false; - switch (target.get_protocol_state(null)) { + switch (target.get_protocol_state()) { case ClientSession.ProtocolState.AUTHORIZED: case ClientSession.ProtocolState.CLOSING_MAILBOX: valid = true; diff --git a/src/engine/imap/transport/imap-client-session.vala b/src/engine/imap/transport/imap-client-session.vala index 32417e5c..fabee175 100644 --- a/src/engine/imap/transport/imap-client-session.vala +++ b/src/engine/imap/transport/imap-client-session.vala @@ -245,13 +245,18 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { get { return this.capabilities.has_capability(Capabilities.IDLE); } } + /** The currently selected mailbox, if any. */ + public MailboxSpecifier? selected_mailbox = null; + /** - * Determines when the last successful command response was received. + * Specifies if the current selected state is readonly. * - * Returns the system wall clock time the last successful command - * response was received, in microseconds since the UNIX epoch. + * This property specifies if the current selected state was + * entered by a SELECT command -- in which case mailbox access is + * read-write, or by an EXAMINE command -- in which case mailbox + * access is read-only. */ - public int64 last_seen = 0; + public bool selected_readonly = false; /** {@inheritDoc} */ public Logging.Flag logging_flags { @@ -262,6 +267,14 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { public Logging.Source? logging_parent { get { return _logging_parent; } } private weak Logging.Source? _logging_parent = null; + /** + * Determines when the last successful command response was received. + * + * Returns the system wall clock time the last successful command + * response was received, in microseconds since the UNIX epoch. + */ + internal int64 last_seen { get; private set; default = 0; } + // While the following inbox and namespace data should be server // specific, there is a small chance they will differ between // connections if the connections connect to different servers in @@ -286,9 +299,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { private Geary.State.Machine fsm; private ClientConnection? cx = null; - private MailboxSpecifier? current_mailbox = null; - private bool current_mailbox_readonly = false; - private uint keepalive_id = 0; private uint selected_keepalive_secs = 0; private uint unselected_keepalive_secs = 0; @@ -333,13 +343,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { public signal void status(StatusData status_data); - /** - * If the mailbox name is null it indicates the type of state change that has occurred - * (authorized -> selected/examined or vice-versa). If new_name is null readonly should be - * ignored. - */ - public signal void current_mailbox_changed(MailboxSpecifier? old_name, MailboxSpecifier? new_name, - bool readonly); public ClientSession(Endpoint imap_endpoint) { this.imap_endpoint = imap_endpoint; @@ -539,14 +542,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { return this.user_namespaces.read_only_view; } - public MailboxSpecifier? get_current_mailbox() { - return current_mailbox; - } - - public bool is_current_mailbox_readonly() { - return current_mailbox_readonly; - } - /** * Determines the SELECT-able mailbox name for a specific folder path. */ @@ -628,9 +623,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { * Returns the current {@link ProtocolState} of the {@link ClientSession} and, if selected, * the current mailbox. */ - public ProtocolState get_protocol_state(out MailboxSpecifier? current_mailbox) { - current_mailbox = null; - + public ProtocolState get_protocol_state() { switch (fsm.get_state()) { case State.NOT_CONNECTED: case State.LOGOUT: @@ -644,8 +637,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { return ProtocolState.AUTHORIZED; case State.SELECTED: - current_mailbox = this.current_mailbox; - return ProtocolState.SELECTED; case State.CONNECTING: @@ -1183,7 +1174,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { public void enable_idle() throws GLib.Error { if (this.is_idle_supported) { - switch (get_protocol_state(null)) { + switch (get_protocol_state()) { case ProtocolState.AUTHORIZING: case ProtocolState.AUTHORIZED: case ProtocolState.SELECTED: @@ -1204,7 +1195,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { unschedule_keepalive(); uint seconds; - switch (get_protocol_state(null)) { + switch (get_protocol_state()) { case ProtocolState.NOT_CONNECTED: case ProtocolState.CONNECTING: return; @@ -1429,45 +1420,34 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { } private uint on_selecting_recv_completion(uint state, uint event, void *user, Object? object) { + uint new_state = state; StatusResponse completion_response = (StatusResponse) object; - Command? cmd; - if (!validate_state_change_cmd(completion_response, out cmd)) - return state; - - // get the mailbox from the command - MailboxSpecifier? mailbox = null; - if (cmd is SelectCommand) { - mailbox = ((SelectCommand) cmd).mailbox; - current_mailbox_readonly = false; - } else if (cmd is ExamineCommand) { - mailbox = ((ExamineCommand) cmd).mailbox; - current_mailbox_readonly = true; - } - - // should only get to this point if cmd was SELECT or EXAMINE - assert(mailbox != null); - - switch (completion_response.status) { + Command? cmd = null; + if (validate_state_change_cmd(completion_response, out cmd)) { + switch (completion_response.status) { case Status.OK: - // mailbox is SELECTED/EXAMINED, report change after completion of transition - MailboxSpecifier? old_mailbox = current_mailbox; - current_mailbox = mailbox; - - if (old_mailbox != current_mailbox) - fsm.do_post_transition(notify_select_completed, null, old_mailbox); - - return State.SELECTED; + if (cmd is SelectCommand) { + this.selected_mailbox = ((SelectCommand) cmd).mailbox; + this.selected_readonly = false; + } else if (cmd is ExamineCommand) { + this.selected_mailbox = ((ExamineCommand) cmd).mailbox; + this.selected_readonly = true; + } + new_state = State.SELECTED; + break; default: + this.selected_mailbox = null; + this.selected_readonly = false; + new_state = State.AUTHORIZED; warning("SELECT/EXAMINE failed: %s", completion_response.to_string()); - return State.AUTHORIZED; + break; + } } - } - private void notify_select_completed(void *user, Object? object) { - current_mailbox_changed((MailboxSpecifier) object, current_mailbox, current_mailbox_readonly); + return new_state; } // @@ -1508,25 +1488,16 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { switch (completion_response.status) { case Status.OK: - MailboxSpecifier? old_mailbox = current_mailbox; - current_mailbox = null; - - if (old_mailbox != null) - fsm.do_post_transition(notify_mailbox_closed, null, old_mailbox); - + this.selected_mailbox = null; + this.selected_readonly = false; return State.AUTHORIZED; default: warning("CLOSE failed: %s", completion_response.to_string()); - return State.SELECTED; } } - private void notify_mailbox_closed(void *user, Object? object) { - current_mailbox_changed((MailboxSpecifier) object, null, false); - } - // // logout // @@ -1645,17 +1616,17 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source { /** {@inheritDoc} */ public Logging.State to_logging_state() { - return (this.current_mailbox == null) + return (this.selected_mailbox == null) ? new Logging.State( this, this.fsm.get_state_string(fsm.get_state()) ) : new Logging.State( this, - "%s:%s %s", + "%s:%s selected %s", this.fsm.get_state_string(fsm.get_state()), - this.current_mailbox.to_string(), - this.current_mailbox_readonly ? "RO" : "RW" + this.selected_mailbox.to_string(), + this.selected_readonly ? "RO" : "RW" ); } diff --git a/test/engine/imap/transport/imap-client-session-test.vala b/test/engine/imap/transport/imap-client-session-test.vala index 8ba46c8a..7ff91357 100644 --- a/test/engine/imap/transport/imap-client-session-test.vala +++ b/test/engine/imap/transport/imap-client-session-test.vala @@ -44,17 +44,17 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(WAIT_FOR_DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.disconnect_async.begin(null, this.async_complete_full); test_article.disconnect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); TestServer.Result result = this.server.wait_for_script(this.main_loop); assert_true( @@ -148,13 +148,13 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(WAIT_FOR_DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.login_async.begin( new Credentials(PASSWORD, "test", "password"), @@ -162,11 +162,11 @@ class Geary.Imap.ClientSessionTest : TestCase { this.async_complete_full ); test_article.login_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == AUTHORIZED); + assert_true(test_article.get_protocol_state() == AUTHORIZED); test_article.disconnect_async.begin(null, this.async_complete_full); test_article.disconnect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); TestServer.Result result = this.server.wait_for_script(this.main_loop); assert_true( @@ -185,17 +185,17 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.logout_async.begin(null, this.async_complete_full); test_article.logout_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); TestServer.Result result = this.server.wait_for_script(this.main_loop); assert_true( @@ -216,13 +216,13 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.login_async.begin( new Credentials(PASSWORD, "test", "password"), @@ -230,11 +230,11 @@ class Geary.Imap.ClientSessionTest : TestCase { this.async_complete_full ); test_article.login_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == AUTHORIZED); + assert_true(test_article.get_protocol_state() == AUTHORIZED); test_article.logout_async.begin(null, this.async_complete_full); test_article.logout_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); TestServer.Result result = this.server.wait_for_script(this.main_loop); assert_true( @@ -261,13 +261,13 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(WAIT_FOR_DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.initiate_session_async.begin( new Credentials(PASSWORD, "test", "password"), @@ -305,13 +305,13 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(WAIT_FOR_DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.initiate_session_async.begin( new Credentials(PASSWORD, "test", "password"), @@ -368,13 +368,13 @@ class Geary.Imap.ClientSessionTest : TestCase { this.server.add_script_line(WAIT_FOR_DISCONNECT, ""); var test_article = new ClientSession(new_endpoint()); - assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED); + assert_true(test_article.get_protocol_state() == NOT_CONNECTED); test_article.connect_async.begin( CONNECT_TIMEOUT, null, this.async_complete_full ); test_article.connect_async.end(async_result()); - assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED); + assert_true(test_article.get_protocol_state() == UNAUTHORIZED); test_article.initiate_session_async.begin( new Credentials(PASSWORD, "test", "password"), diff --git a/test/integration/imap/client-session.vala b/test/integration/imap/client-session.vala index 7bafd9f4..f6221f63 100644 --- a/test/integration/imap/client-session.vala +++ b/test/integration/imap/client-session.vala @@ -33,7 +33,7 @@ class Integration.Imap.ClientSession : TestCase { } public override void tear_down() throws GLib.Error { - if (this.session.get_protocol_state(null) != NOT_CONNECTED) { + if (this.session.get_protocol_state() != NOT_CONNECTED) { this.session.disconnect_async.begin(null, async_complete_full); this.session.disconnect_async.end(async_result()); }