[geary/mjog/imap-connection-fixes: 37/42] Simply Geary.Imap.ClientService selected mailbox handling
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/mjog/imap-connection-fixes: 37/42] Simply Geary.Imap.ClientService selected mailbox handling
- Date: Wed, 25 Mar 2020 22:03:18 +0000 (UTC)
commit 55940a98c27d20aa3867010b4fda974dc4ae5f72
Author: Michael Gratton <mike vee net>
Date: Mon Dec 30 15:03:56 2019 +1030
Simply Geary.Imap.ClientService selected mailbox handling
Convert mailbox and mailbox RW/RO state accessors to properties and
rename to be more explicit about what they mean. Remove mailbox arg
from ::getProtocolState() since it's unused.
src/engine/imap/api/imap-client-service.vala | 6 +-
src/engine/imap/transport/imap-client-session.vala | 119 ++++++++-------------
.../imap/transport/imap-client-session-test.vala | 40 +++----
test/integration/imap/client-session.vala | 2 +-
4 files changed, 69 insertions(+), 98 deletions(-)
---
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());
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
