Merge pull request #531 from jberkel/imap-tests

Update greenmail + add more tests

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapConnection.java

@@ -601,10 +601,18 @@ class ImapConnection {
     }
 
     private List<ImapResponse> receiveCapabilities(List<ImapResponse> responses) {
-        capabilities = ImapResponseParser.parseCapabilities(responses);
+        Set<String> receivedCapabilities = ImapResponseParser.parseCapabilities(responses);
+        /* RFC 3501 6.2.3
+            A server MAY include a CAPABILITY response code in the tagged OK
+            response to a successful LOGIN command in order to send
+            capabilities automatically.  It is unnecessary for a client to
+            send a separate CAPABILITY command if it recognizes these
+            automatic capabilities.
+        */
         if (K9MailLib.isDebug()) {
-            Log.d(LOG_TAG, "Saving " + capabilities + " capabilities for " + getLogId());
+            Log.d(LOG_TAG, "Saving " + receivedCapabilities + " capabilities for " + getLogId());
         }
+        capabilities.addAll(receivedCapabilities);
         return responses;
     }
 }

k9mail/build.gradle

@@ -5,6 +5,9 @@ apply from: '../gradle/plugins/findbugs-android.gradle'
 
 repositories {
     jcenter()
+    maven {
+        url "https://oss.sonatype.org/content/repositories/snapshots/"
+    }
 }
 
 dependencies {
@@ -21,9 +24,10 @@ dependencies {
     androidTestCompile 'com.android.support.test:testing-support-lib:0.1'
     androidTestCompile 'com.android.support.test.espresso:espresso-core:2.0'
 
-    androidTestCompile("com.icegreen:greenmail:1.3.1b") {
+    androidTestCompile("com.icegreen:greenmail:1.4.1-SNAPSHOT") {
         // Use a better, later version
-        exclude group: "javax.mail"
+        exclude group: "com.sun.mail"
+        exclude group: "junit"
     }
 
     // this version avoids some "Ignoring InnerClasses attribute for an anonymous inner class" warnings

k9mail/src/androidTest/java/com/fsck/k9/endtoend/AbstractEndToEndTest.java

@@ -37,6 +37,12 @@ public abstract class AbstractEndToEndTest<T extends Activity> extends ActivityI
         if (bypassWelcome) {
             bypassWelcomeScreen();
         }
+        state.stubMailServer = new StubMailServer();
+    }
+
+    @Override
+    public void tearDown() throws Exception {
+        state.stubMailServer.stop();
     }
 
     private void bypassWelcomeScreen() {
@@ -51,10 +57,7 @@ public abstract class AbstractEndToEndTest<T extends Activity> extends ActivityI
         }
     }
 
-    protected StubMailServer setupMailServer() {
+    public StubMailServer stubMailServer() {
-        if (null == state.stubMailServer) {
-            state.stubMailServer = new StubMailServer();
-        }
         return state.stubMailServer;
     }
 }

k9mail/src/androidTest/java/com/fsck/k9/endtoend/AccountSetupFlow.java

@@ -45,11 +45,9 @@ public class AccountSetupFlow {
 
         IncomingServerSettingsPage incoming = accountTypePage.clickImap();
 
-        StubMailServer stubMailServer = test.setupMailServer();
+        OutgoingServerSettingsPage outgoing = setupIncomingServerAndClickNext(incoming, test.stubMailServer());
 
-        OutgoingServerSettingsPage outgoing = setupIncomingServerAndClickNext(incoming, stubMailServer);
+        AccountOptionsPage accountOptionsPage = setupOutgoingServerAndClickNext(outgoing, test.stubMailServer());
-
-        AccountOptionsPage accountOptionsPage = setupOutgoingServerAndClickNext(outgoing, stubMailServer);
 
         AccountSetupNamesPage accountSetupNamesPage = accountOptionsPage.clickNext();
 
@@ -61,7 +59,7 @@ public class AccountSetupFlow {
 
         accountsPage.assertAccountExists(accountDescription);
 
-        ApplicationState.getInstance().accounts.add(new AccountForTest(ACCOUNT_NAME, accountDescription, stubMailServer));
+        ApplicationState.getInstance().accounts.add(new AccountForTest(ACCOUNT_NAME, accountDescription, test.stubMailServer()));
 
         return accountsPage;
     }

k9mail/src/androidTest/java/com/fsck/k9/endtoend/framework/StubMailServer.java

@@ -37,5 +37,9 @@ public class StubMailServer {
     public int getImapPort() {
         return IMAP_SERVER_SETUP.getPort();
     }
+
+    public void stop() {
+        greenmail.stop();
+    }
 }
 

k9mail/src/androidTest/java/com/fsck/k9/mail/store/imap/ImapConnectionTest.java

@@ -0,0 +1,196 @@
+package com.fsck.k9.mail.store.imap;
+
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import com.fsck.k9.endtoend.framework.StubMailServer;
+import com.fsck.k9.endtoend.framework.UserForImap;
+import com.fsck.k9.mail.AuthType;
+import com.fsck.k9.mail.AuthenticationFailedException;
+import com.fsck.k9.mail.ConnectionSecurity;
+import com.fsck.k9.mail.MessagingException;
+import junit.framework.TestCase;
+
+import static org.junit.Assert.assertArrayEquals;
+
+
+public class ImapConnectionTest extends TestCase {
+    private static final String[] CAPABILITIES = new String[] { "IMAP4REV1", "LITERAL+", "QUOTA" };
+
+    private StubMailServer stubMailServer;
+    private ImapConnection connection;
+    private TestImapSettings settings;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        stubMailServer = new StubMailServer();
+        settings = new TestImapSettings(UserForImap.TEST_USER);
+        connection = new ImapConnection(settings, null, null);
+    }
+
+    @Override
+    public void tearDown() throws Exception {
+        super.tearDown();
+        stubMailServer.stop();
+    }
+
+    public void testOpenConnectionWithoutRunningServerThrowsMessagingException() throws Exception {
+        stubMailServer.stop();
+        try {
+            connection.open();
+            fail("expected exception");
+        } catch (MessagingException e) {
+            assertFalse(connection.isOpen());
+        }
+    }
+
+    public void testOpenConnectionWithWrongCredentialsThrowsAuthenticationFailedException() throws Exception {
+        connection = new ImapConnection(new TestImapSettings("wrong", "password"), null, null);
+        try {
+            connection.open();
+            fail("expected exception");
+        } catch (AuthenticationFailedException e) {
+            assertTrue(e.getMessage().contains("Invalid login/password"));
+            assertFalse(connection.isOpen());
+        }
+    }
+
+    public void testConnectionIsInitiallyClosed() throws Exception {
+        assertFalse(connection.isOpen());
+    }
+
+    public void testSuccessfulOpenConnectionTogglesOpenState() throws Exception {
+        connection.open();
+        assertTrue(connection.isOpen());
+    }
+
+    public void testSuccessfulOpenAndCloseConnectionTogglesOpenState() throws Exception {
+        connection.open();
+        connection.close();
+        assertFalse(connection.isOpen());
+    }
+
+    public void testCapabilitiesAreInitiallyEmpty() throws Exception {
+        assertTrue(connection.getCapabilities().isEmpty());
+    }
+
+    public void testCapabilitiesListGetsParsedCorrectly() throws Exception {
+        connection.open();
+        List<String> capabilities = new ArrayList<String>(connection.getCapabilities());
+        Collections.sort(capabilities);
+        assertArrayEquals(CAPABILITIES, capabilities.toArray());
+    }
+
+    public void testHasCapabilityChecks() throws Exception {
+        connection.open();
+        for (String capability : CAPABILITIES) {
+            assertTrue(connection.hasCapability(capability));
+        }
+        assertFalse(connection.hasCapability("FROBAZIFCATE"));
+    }
+
+    public void testPathPrefixGetsSetCorrectly() throws Exception {
+        connection.open();
+        assertEquals("", settings.getPathPrefix());
+    }
+
+    public void testPathDelimiterGetsParsedCorrectly() throws Exception {
+        connection.open();
+        assertEquals(".", settings.getPathDelimiter());
+    }
+
+    public void testCombinedPrefixGetsSetCorrectly() throws Exception {
+        connection.open();
+        assertNull(settings.getCombinedPrefix());
+    }
+
+    private class TestImapSettings implements ImapSettings {
+        private String pathPrefix;
+        private String pathDelimiter;
+        private String username;
+        private String password;
+        private String combinedPrefix;
+
+        public TestImapSettings(UserForImap userForImap) {
+            this(userForImap.loginUsername, userForImap.password);
+        }
+
+        public TestImapSettings(String username, String password) {
+            this.username = username;
+            this.password = password;
+        }
+
+        @Override
+        public String getHost() {
+            return stubMailServer.getImapBindAddress();
+        }
+
+        @Override
+        public int getPort() {
+            return stubMailServer.getImapPort();
+        }
+
+        @Override
+        public ConnectionSecurity getConnectionSecurity() {
+            return ConnectionSecurity.NONE;
+        }
+
+        @Override
+        public AuthType getAuthType() {
+            return AuthType.PLAIN;
+        }
+
+        @Override
+        public String getUsername() {
+            return username;
+        }
+
+        @Override
+        public String getPassword() {
+            return password;
+        }
+
+        @Override
+        public String getClientCertificateAlias() {
+            return null;
+        }
+
+        @Override
+        public boolean useCompression(int type) {
+            return false;
+        }
+
+        @Override
+        public String getPathPrefix() {
+            return pathPrefix;
+        }
+
+        @Override
+        public void setPathPrefix(String prefix) {
+            pathPrefix = prefix;
+        }
+
+        @Override
+        public String getPathDelimiter() {
+            return pathDelimiter;
+        }
+
+        @Override
+        public void setPathDelimiter(String delimiter) {
+            pathDelimiter = delimiter;
+        }
+
+        @Override
+        public String getCombinedPrefix() {
+            return combinedPrefix;
+        }
+
+        @Override
+        public void setCombinedPrefix(String prefix) {
+            combinedPrefix = prefix;
+        }
+    }
+}

k9mail/src/main/java/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java

@@ -8,13 +8,12 @@ import android.app.FragmentTransaction;
 import android.content.DialogInterface;
 import android.content.Intent;
 import android.net.Uri;
+import android.os.AsyncTask;
 import android.os.Bundle;
 import android.os.Handler;
-import android.os.Process;
 import android.util.Log;
 import android.view.View;
 import android.view.View.OnClickListener;
-import android.widget.Button;
 import android.widget.ProgressBar;
 import android.widget.TextView;
 
@@ -87,7 +86,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
         setContentView(R.layout.account_setup_check_settings);
         mMessageView = (TextView)findViewById(R.id.message);
         mProgressBar = (ProgressBar)findViewById(R.id.progress);
-        ((Button)findViewById(R.id.cancel)).setOnClickListener(this);
+        findViewById(R.id.cancel).setOnClickListener(this);
 
         setMessage(R.string.account_setup_check_settings_retr_info_msg);
         mProgressBar.setIndeterminate(true);
@@ -96,82 +95,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
         mAccount = Preferences.getPreferences(this).getAccount(accountUuid);
         mDirection = (CheckDirection) getIntent().getSerializableExtra(EXTRA_CHECK_DIRECTION);
 
-        new Thread() {
+        new CheckAccountTask(mAccount).execute(mDirection);
-            @Override
-            public void run() {
-                Store store = null;
-                Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
-                try {
-                    if (mDestroyed) {
-                        return;
-                    }
-                    if (mCanceled) {
-                        finish();
-                        return;
-                    }
-
-                    final MessagingController ctrl = MessagingController.getInstance(getApplication());
-                    ctrl.clearCertificateErrorNotifications(AccountSetupCheckSettings.this,
-                            mAccount, mDirection);
-
-                    if (mDirection == CheckDirection.INCOMING) {
-                        store = mAccount.getRemoteStore();
-
-                        if (store instanceof WebDavStore) {
-                            setMessage(R.string.account_setup_check_settings_authenticate);
-                        } else {
-                            setMessage(R.string.account_setup_check_settings_check_incoming_msg);
-                        }
-                        store.checkSettings();
-
-                        if (store instanceof WebDavStore) {
-                            setMessage(R.string.account_setup_check_settings_fetch);
-                        }
-                        MessagingController.getInstance(getApplication()).listFoldersSynchronous(mAccount, true, null);
-                        MessagingController.getInstance(getApplication()).synchronizeMailbox(mAccount, mAccount.getInboxFolderName(), null, null);
-                    }
-                    if (mDestroyed) {
-                        return;
-                    }
-                    if (mCanceled) {
-                        finish();
-                        return;
-                    }
-                    if (mDirection == CheckDirection.OUTGOING) {
-                        if (!(mAccount.getRemoteStore() instanceof WebDavStore)) {
-                            setMessage(R.string.account_setup_check_settings_check_outgoing_msg);
-                        }
-                        Transport transport = Transport.getInstance(K9.app, mAccount);
-                        transport.close();
-                        transport.open();
-                        transport.close();
-                    }
-                    if (mDestroyed) {
-                        return;
-                    }
-                    if (mCanceled) {
-                        finish();
-                        return;
-                    }
-                    setResult(RESULT_OK);
-                    finish();
-                } catch (final AuthenticationFailedException afe) {
-                    Log.e(K9.LOG_TAG, "Error while testing settings", afe);
-                    showErrorDialog(
-                        R.string.account_setup_failed_dlg_auth_message_fmt,
-                        afe.getMessage() == null ? "" : afe.getMessage());
-                } catch (final CertificateValidationException cve) {
-                    handleCertificateValidationException(cve);
-                } catch (final Throwable t) {
-                    Log.e(K9.LOG_TAG, "Error while testing settings", t);
-                    showErrorDialog(
-                        R.string.account_setup_failed_dlg_server_message_fmt,
-                        (t.getMessage() == null ? "" : t.getMessage()));
-                }
-            }
-
-        }
-        .start();
     }
 
     private void handleCertificateValidationException(CertificateValidationException cve) {
@@ -199,14 +123,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
     }
 
     private void setMessage(final int resId) {
-        mHandler.post(new Runnable() {
+        mMessageView.setText(getString(resId));
-            public void run() {
-                if (mDestroyed) {
-                    return;
-                }
-                mMessageView.setText(getString(resId));
-            }
-        });
     }
 
     private void acceptKeyDialog(final int msgResId, final CertificateValidationException ex) {
@@ -266,7 +183,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
                             for (List<?> subjectAlternativeName : subjectAlternativeNames) {
                                 Integer type = (Integer)subjectAlternativeName.get(0);
                                 Object value = subjectAlternativeName.get(1);
-                                String name = "";
+                                String name;
                                 switch (type.intValue()) {
                                 case 0:
                                     Log.w(K9.LOG_TAG, "SubjectAltName of type OtherName not supported.");
@@ -473,4 +390,89 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
             default: return "";
         }
     }
+
+    class CheckAccountTask extends AsyncTask<CheckDirection, Integer, Void> {
+        private final Account account;
+
+        CheckAccountTask(Account account) {
+            this.account = account;
+        }
+
+        @Override
+        protected Void doInBackground(CheckDirection... params) {
+            final CheckDirection direction = params[0];
+            try {
+                if (mDestroyed) {
+                    return null;
+                }
+                if (mCanceled) {
+                    finish();
+                    return null;
+                }
+                final MessagingController ctrl = MessagingController.getInstance(getApplication());
+                ctrl.clearCertificateErrorNotifications(AccountSetupCheckSettings.this,
+                        account, direction);
+
+                if (direction == CheckDirection.INCOMING) {
+                    Store store = account.getRemoteStore();
+                    if (store instanceof WebDavStore) {
+                        publishProgress(R.string.account_setup_check_settings_authenticate);
+                    } else {
+                        publishProgress(R.string.account_setup_check_settings_check_incoming_msg);
+                    }
+                    store.checkSettings();
+
+                    if (store instanceof WebDavStore) {
+                        publishProgress(R.string.account_setup_check_settings_fetch);
+                    }
+                    MessagingController.getInstance(getApplication()).listFoldersSynchronous(account, true, null);
+                    MessagingController.getInstance(getApplication())
+                            .synchronizeMailbox(account, account.getInboxFolderName(), null, null);
+                }
+                if (mDestroyed) {
+                    return null;
+                }
+                if (mCanceled) {
+                    finish();
+                    return null;
+                }
+                if (direction == CheckDirection.OUTGOING) {
+                    if (!(account.getRemoteStore() instanceof WebDavStore)) {
+                        publishProgress(R.string.account_setup_check_settings_check_outgoing_msg);
+                    }
+                    Transport transport = Transport.getInstance(K9.app, account);
+                    transport.close();
+                    transport.open();
+                    transport.close();
+                }
+                if (mDestroyed) {
+                    return null;
+                }
+                if (mCanceled) {
+                    finish();
+                    return null;
+                }
+                setResult(RESULT_OK);
+                finish();
+            } catch (AuthenticationFailedException afe) {
+                Log.e(K9.LOG_TAG, "Error while testing settings", afe);
+                showErrorDialog(
+                        R.string.account_setup_failed_dlg_auth_message_fmt,
+                        afe.getMessage() == null ? "" : afe.getMessage());
+            } catch (CertificateValidationException cve) {
+                handleCertificateValidationException(cve);
+            } catch (Throwable t) {
+                Log.e(K9.LOG_TAG, "Error while testing settings", t);
+                showErrorDialog(
+                        R.string.account_setup_failed_dlg_server_message_fmt,
+                        (t.getMessage() == null ? "" : t.getMessage()));
+            }
+            return null;
+        }
+
+        @Override
+        protected void onProgressUpdate(Integer... values) {
+            setMessage(values[0]);
+        }
+    }
 }