[geary] More explicit ClientSession.ProtocolState



commit f27f0c079c8407dd4d546b36d961473074f2333d
Author: Jim Nelson <jim yorba org>
Date:   Thu Mar 12 17:02:08 2015 -0700

    More explicit ClientSession.ProtocolState
    
    Some code was playing loosey-goosey with the ClientSession IN_PROGRESS
    Context, which was a general value for "in transition" without
    specifying what transition.  Now Context (renamed ProtocolState) is
    explicit about the transition and the affected code is more selective
    about what action it takes in response.

 src/engine/api/geary-engine.vala                   |    2 +-
 src/engine/imap/api/imap-account.vala              |    4 +-
 .../transport/imap-client-session-manager.vala     |   22 ++++---
 src/engine/imap/transport/imap-client-session.vala |   63 +++++++++++++++-----
 4 files changed, 64 insertions(+), 27 deletions(-)
---
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index 14f3d9d..0a5ce05 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -280,7 +280,7 @@ public class Geary.Engine : BaseObject {
                 
                 // Connected and initiated, still need to be sure connection authorized
                 Imap.MailboxSpecifier current_mailbox;
-                if (imap_session.get_context(out current_mailbox) != Imap.ClientSession.Context.AUTHORIZED)
+                if (imap_session.get_protocol_state(out current_mailbox) != 
Imap.ClientSession.ProtocolState.AUTHORIZED)
                     error_code |= ValidationResult.IMAP_CREDENTIALS_INVALID;
             } catch (Error err) {
                 debug("Error validating IMAP account info: %s", err.message);
diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala
index 9b6abca..8d6aefb 100644
--- a/src/engine/imap/api/imap-account.vala
+++ b/src/engine/imap/api/imap-account.vala
@@ -79,8 +79,10 @@ private class Geary.Imap.Account : BaseObject {
     // possibly is long enough for ClientSessionManager to get a few ready).
     private async ClientSession claim_session_async(Cancellable? cancellable) throws Error {
         // check if available session is in good state
-        if (account_session != null && account_session.get_context(null) != ClientSession.Context.AUTHORIZED)
+        if (account_session != null
+            && account_session.get_protocol_state(null) != ClientSession.ProtocolState.AUTHORIZED) {
             yield drop_session_async(cancellable);
+        }
         
         int token = yield account_session_mutex.claim_async(cancellable);
         
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala 
b/src/engine/imap/transport/imap-client-session-manager.vala
index ecbc1ce..73be91a 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -310,7 +310,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
         foreach (ClientSession session in sessions) {
             MailboxSpecifier? mailbox;
             if (!reserved_sessions.contains(session) &&
-                (session.get_context(out mailbox) == ClientSession.Context.AUTHORIZED)) {
+                (session.get_protocol_state(out mailbox) == ClientSession.ProtocolState.AUTHORIZED)) {
                 found_session = session;
                 
                 break;
@@ -350,26 +350,30 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
         // during mop-up
         
         MailboxSpecifier? mailbox;
-        ClientSession.Context context = session.get_context(out mailbox);
+        ClientSession.ProtocolState context = session.get_protocol_state(out mailbox);
         
         bool unreserve = false;
         switch (context) {
-            case ClientSession.Context.AUTHORIZED:
+            case ClientSession.ProtocolState.AUTHORIZED:
+            case ClientSession.ProtocolState.CLOSING_MAILBOX:
                 // keep as-is, but remove from the reserved list
                 unreserve = true;
             break;
             
-            case ClientSession.Context.UNAUTHORIZED:
+            // ClientSessionManager is tasked with holding onto a pool of authorized connections,
+            // so if one is released outside that state, pessimistically drop it
+            case ClientSession.ProtocolState.CONNECTING:
+            case ClientSession.ProtocolState.AUTHORIZING:
+            case ClientSession.ProtocolState.UNAUTHORIZED:
                 yield force_disconnect_async(session, true);
             break;
             
-            case ClientSession.Context.UNCONNECTED:
+            case ClientSession.ProtocolState.UNCONNECTED:
                 yield force_disconnect_async(session, false);
             break;
             
-            case ClientSession.Context.IN_PROGRESS:
-            case ClientSession.Context.EXAMINED:
-            case ClientSession.Context.SELECTED:
+            case ClientSession.ProtocolState.SELECTED:
+            case ClientSession.ProtocolState.SELECTING:
                 // always close mailbox to return to authorized state
                 try {
                     yield session.close_mailbox_async(cancellable);
@@ -379,7 +383,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
                 }
                 
                 // if not in authorized state now, drop it, otherwise remove from reserved list
-                if (session.get_context(out mailbox) == ClientSession.Context.AUTHORIZED)
+                if (session.get_protocol_state(out mailbox) == ClientSession.ProtocolState.AUTHORIZED)
                     unreserve = true;
                 else
                     yield force_disconnect_async(session, true);
diff --git a/src/engine/imap/transport/imap-client-session.vala 
b/src/engine/imap/transport/imap-client-session.vala
index 2501f7f..f715ae4 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -24,13 +24,32 @@ public class Geary.Imap.ClientSession : BaseObject {
     
     private const uint GREETING_TIMEOUT_SEC = ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC;
     
-    public enum Context {
+    /**
+     * The various states an IMAP { link ClientSession} may be in at any moment.
+     *
+     * These don't exactly match the states in the IMAP specification.  For one, they count
+     * transitions as states unto themselves (due to network latency and the asynchronous nature
+     * of ClientSession's interface).  Also, the LOGOUT (and logging out) state has been melded
+     * into { link ProtocolState.UNCONNECTED} on the presumption that the nuances of a disconnected or
+     * disconnecting session is uninteresting to the caller.
+     *
+     * See [[http://tools.ietf.org/html/rfc3501#section-3]]
+     *
+     * @see get_protocol_state
+     */
+    public enum ProtocolState {
         UNCONNECTED,
+        CONNECTING,
         UNAUTHORIZED,
+        AUTHORIZING,
         AUTHORIZED,
+        SELECTING,
         SELECTED,
-        EXAMINED,
-        IN_PROGRESS
+        /**
+         * Indicates the { link ClientSession} is closing a ''mailbox'', i.e. a folder, not the
+         * connection itself.
+         */
+        CLOSING_MAILBOX
     }
     
     public enum DisconnectReason {
@@ -395,7 +414,11 @@ public class Geary.Imap.ClientSession : BaseObject {
         return current_mailbox_readonly;
     }
     
-    public Context get_context(out MailboxSpecifier? current_mailbox) {
+    /**
+     * 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;
         
         switch (fsm.get_state()) {
@@ -403,24 +426,30 @@ public class Geary.Imap.ClientSession : BaseObject {
             case State.LOGGED_OUT:
             case State.LOGGING_OUT:
             case State.BROKEN:
-                return Context.UNCONNECTED;
+                return ProtocolState.UNCONNECTED;
             
             case State.NOAUTH:
-                return Context.UNAUTHORIZED;
+                return ProtocolState.UNAUTHORIZED;
             
             case State.AUTHORIZED:
-                return Context.AUTHORIZED;
+                return ProtocolState.AUTHORIZED;
             
             case State.SELECTED:
                 current_mailbox = this.current_mailbox;
                 
-                return current_mailbox_readonly ? Context.EXAMINED : Context.SELECTED;
+                return ProtocolState.SELECTED;
             
             case State.CONNECTING:
+                return ProtocolState.CONNECTING;
+            
             case State.AUTHORIZING:
+                return ProtocolState.AUTHORIZING;
+            
             case State.SELECTING:
+                return ProtocolState.SELECTING;
+            
             case State.CLOSING_MAILBOX:
-                return Context.IN_PROGRESS;
+                return ProtocolState.CLOSING_MAILBOX;
             
             default:
                 assert_not_reached();
@@ -851,19 +880,21 @@ public class Geary.Imap.ClientSession : BaseObject {
         unschedule_keepalive();
         
         uint seconds;
-        switch (get_context(null)) {
-            case Context.UNCONNECTED:
+        switch (get_protocol_state(null)) {
+            case ProtocolState.UNCONNECTED:
+            case ProtocolState.CONNECTING:
                 return;
             
-            case Context.IN_PROGRESS:
-            case Context.EXAMINED:
-            case Context.SELECTED:
+            case ProtocolState.SELECTING:
+            case ProtocolState.SELECTED:
                 seconds = (allow_idle && supports_idle()) ? selected_with_idle_keepalive_secs
                     : selected_keepalive_secs;
             break;
             
-            case Context.UNAUTHORIZED:
-            case Context.AUTHORIZED:
+            case ProtocolState.UNAUTHORIZED:
+            case ProtocolState.AUTHORIZING:
+            case ProtocolState.AUTHORIZED:
+            case ProtocolState.CLOSING_MAILBOX:
             default:
                 seconds = unselected_keepalive_secs;
             break;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]