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

Simply Geary.Imap.ClientService selected mailbox handling

Browse source 
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.
This commit is contained in:
Michael Gratton 2019-12-30 15:03:56 +10:30 • committed by Michael James Gratton
parent 9e01d8dca0
commit b53d5b2470
4 changed files with 69 additions and 98 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/engine/imap/api/imap-client-service.vala
View file

@ -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;

119
src/engine/imap/transport/imap-client-session.vala
View file

@ -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"
);
}

40
test/engine/imap/transport/imap-client-session-test.vala
View file

@ -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"),

2
test/integration/imap/client-session.vala
View file

@ -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());
}