Merge branch 'wip/156-mailbox-not-exist' into 'master'

No folders listed when STATUS returns not found for a child

See merge request GNOME/geary!189

src/engine/imap/command/imap-command.vala

@@ -243,9 +243,17 @@ public class Geary.Imap.Command : BaseObject {
             );
         }
 
+        check_has_status();
+
         // Since this is part of the public API, perform a strict
         // check on the status code.
-        check_status(true);
+        if (this.status.status == Status.BAD) {
+            throw new ImapError.SERVER_ERROR(
+                "%s: Command failed: %s",
+                to_brief_string(),
+                this.status.to_string()
+            );
+        }
     }
 
     public virtual string to_string() {
@@ -276,9 +284,8 @@ public class Geary.Imap.Command : BaseObject {
         this.response_timer.reset();
         this.complete_lock.blind_notify();
         cancel_send();
-        // Since this gets called by the client connection only check
-        // for an expected server response, good or bad
-        check_status(false);
+
+        check_has_status();
     }
 
     /**
@@ -340,7 +347,7 @@ public class Geary.Imap.Command : BaseObject {
         }
     }
 
-    private void check_status(bool require_okay) throws ImapError {
+    private void check_has_status() throws ImapError {
         if (this.status == null) {
             throw new ImapError.SERVER_ERROR(
                 "%s: No command response was received",
@@ -355,16 +362,6 @@ public class Geary.Imap.Command : BaseObject {
                 this.status.to_string()
             );
         }
-
-        // XXX should we be distinguishing between NO and BAD
-        // responses here?
-        if (require_okay && this.status.status != Status.OK) {
-            throw new ImapError.SERVER_ERROR(
-                "%s: Command failed: %s",
-                to_brief_string(),
-                this.status.to_string()
-            );
-        }
     }
 
     private string to_brief_string() {

src/engine/imap/transport/imap-client-session.vala

@@ -835,6 +835,10 @@ public class Geary.Imap.ClientSession : BaseObject {
     /**
      * Performs the LOGIN command using the supplied credentials.
      *
+     * Throws {@link ImapError.UNAUTHENTICATED} if the credentials are
+     * bad, unsupported, or if authentication actually failed. Returns
+     * the status response for the command otherwise.
+     *
      * @see initiate_session_async
      */
     public async StatusResponse login_async(Geary.Credentials credentials,
@@ -877,32 +881,40 @@ public class Geary.Imap.ClientSession : BaseObject {
         // should always proceed; only an Error could change this
         assert(params.proceed);
 
-        GLib.Error? login_err = null;
-        try {
-            yield command_transaction_async(cmd, cancellable);
-        } catch (Error err) {
-            login_err = err;
-        }
+        StatusResponse response = yield command_transaction_async(
+            cmd, cancellable
+        );
 
-        if (login_err != null) {
+        if (response.status != Status.OK) {
             // Throw an error indicating auth failed here, unless
             // there is a status response and it indicates that the
             // server is merely reporting login as being unavailable,
             // then don't since the creds might actually be fine.
-            ResponseCodeType? code_type = null;
-            if (cmd.status != null) {
-                ResponseCode? code = cmd.status.response_code;
-                if (code != null) {
-                    code_type = code.get_response_code_type();
+            ResponseCode? code = response.response_code;
+            if (code != null) {
+                ResponseCodeType? code_type = code.get_response_code_type();
+                if (code_type != null) {
+                    switch (code_type.value) {
+                    case ResponseCodeType.UNAVAILABLE:
+                        throw new ImapError.UNAVAILABLE(
+                            "Login restricted: %s: ", response.to_string()
+                        );
+
+                    case ResponseCodeType.AUTHENTICATIONFAILED:
+                        // pass through to the error being thrown below
+                        break;
+
+                    default:
+                        throw new ImapError.SERVER_ERROR(
+                            "Login error: %s: ", response.to_string()
+                        );
+                    }
                 }
             }
 
-            if (code_type == null ||
-                code_type.value != ResponseCodeType.UNAVAILABLE) {
-                throw new ImapError.UNAUTHENTICATED(login_err.message);
-            } else {
-                throw login_err;
-            }
+            throw new ImapError.UNAUTHENTICATED(
+                "Bad credentials: %s: ", response.to_string()
+            );
         }
 
         return cmd.status;
@@ -1715,6 +1727,9 @@ public class Geary.Imap.ClientSession : BaseObject {
 
         this.cx.send_command(cmd);
         yield cmd.wait_until_complete(cancellable);
+
+        // This won't be null since the Command.wait_until_complete
+        // will throw an error if it is.
         return cmd.status;
     }
 

test/integration/imap/client-session.vala

@@ -62,6 +62,12 @@ class Integration.Imap.ClientSession : TestCase {
             assert_not_reached();
         } catch (Geary.ImapError.UNAUTHENTICATED err) {
             // All good
+        } catch (Geary.ImapError.SERVER_ERROR err) {
+            // Some servers (Y!) return AUTHORIZATIONFAILED response
+            // code if the login (not password) is bad
+            if (!("AUTHORIZATIONFAILED" in err.message)) {
+                throw err;
+            }
         }
     }