diff --git a/src/com/android/email/mail/store/ImapStore.java b/src/com/android/email/mail/store/ImapStore.java index 7e2bc8679..c96cd532c 100644 --- a/src/com/android/email/mail/store/ImapStore.java +++ b/src/com/android/email/mail/store/ImapStore.java @@ -510,10 +510,15 @@ public class ImapStore extends Store { mName = name; } + private void destroyResponses() { + if (mConnection != null) { + mConnection.destroyResponses(); + } + } + @Override public void open(OpenMode mode, PersistentDataCallbacks callbacks) throws MessagingException { - try { if (isOpen()) { if (mMode == mode) { @@ -526,7 +531,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } else { // Return the connection to the pool, if exists. @@ -582,7 +587,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } catch (MessagingException e) { mExists = false; @@ -606,6 +611,7 @@ public class ImapStore extends Store { // TODO implement expunge mMessageCount = -1; synchronized (this) { + destroyResponses(); mStore.poolConnection(mConnection); mConnection = null; } @@ -707,7 +713,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } @@ -735,7 +741,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } @@ -773,7 +779,7 @@ public class ImapStore extends Store { } return uids.toArray(Utility.EMPTY_STRINGS); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } @@ -835,16 +841,16 @@ public class ImapStore extends Store { fetchInternal(messages, fp, listener); } catch (RuntimeException e) { // Probably a parser error. Log.w(Email.LOG_TAG, "Exception detected: " + e.getMessage()); - mConnection.logLastDiscourse(); + if (mConnection != null) { + mConnection.logLastDiscourse(); + } throw e; - } finally { - mConnection.destroyResponses(); } } public void fetchInternal(Message[] messages, FetchProfile fp, MessageRetrievalListener listener) throws MessagingException { - if (messages == null || messages.length == 0) { + if (messages.length == 0) { return; } checkOpen(); @@ -897,7 +903,7 @@ public class ImapStore extends Store { } try { - String tag = mConnection.sendCommand(String.format( + mConnection.sendCommand(String.format( ImapConstants.UID_FETCH + " %s (%s)", joinMessageUids(messages), Utility.combine(fetchFields.toArray(new String[fetchFields.size()]), ' ') ), false); @@ -991,7 +997,7 @@ public class ImapStore extends Store { listener.messageRetrieved(message); } } finally { - mConnection.destroyResponses(); + destroyResponses(); } } while (!response.isTagged()); } catch (IOException ioe) { @@ -1285,7 +1291,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } @@ -1297,7 +1303,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } return null; } @@ -1332,7 +1338,7 @@ public class ImapStore extends Store { } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { - mConnection.destroyResponses(); + destroyResponses(); } } @@ -1344,8 +1350,15 @@ public class ImapStore extends Store { private MessagingException ioExceptionHandler(ImapConnection connection, IOException ioe) throws MessagingException { + if (Email.DEBUG) { + Log.d(Email.LOG_TAG, "IO Exception detected: ", ioe); + } + connection.destroyResponses(); connection.close(); - close(false); + if (connection == mConnection) { + mConnection = null; // To prevent close() from returning the connection to the pool. + close(false); + } return new MessagingException("IO Error", ioe); } diff --git a/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java b/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java index fbdbfbe30..867e86acf 100644 --- a/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java +++ b/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java @@ -50,6 +50,7 @@ import android.test.AndroidTestCase; import android.test.MoreAsserts; import android.test.suitebuilder.annotation.SmallTest; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.regex.Pattern; @@ -1612,4 +1613,115 @@ public class ImapStoreUnitTests extends AndroidTestCase { folders[1].open(OpenMode.READ_WRITE, null); folders[1].close(false); } + + /** + * Callback for {@link #runAndExpectMessagingException}. + */ + private interface RunAndExpectMessagingExceptionTarget { + public void run(MockTransport mockTransport) throws Exception; + } + + /** + * Set up the usual mock transport, open the folder, + * run {@link RunAndExpectMessagingExceptionTarget} and make sure a {@link MessagingException} + * is thrown. + */ + private void runAndExpectMessagingException(RunAndExpectMessagingExceptionTarget target) + throws Exception { + try { + final MockTransport mockTransport = openAndInjectMockTransport(); + setupOpenFolder(mockTransport); + mFolder.open(OpenMode.READ_WRITE, null); + + target.run(mockTransport); + + fail("MessagingException expected."); + } catch (MessagingException expected) { + } + } + + /** + * Make sure that IOExceptions are always converted to MessagingException. + */ + public void testFetchIOException() throws Exception { + runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() { + public void run(MockTransport mockTransport) throws Exception { + mockTransport.expectIOException(); + + final Message message = mFolder.createMessage("1"); + final FetchProfile fp = new FetchProfile(); + fp.add(FetchProfile.Item.STRUCTURE); + + mFolder.fetch(new Message[] { message }, fp, null); + } + }); + } + + /** + * Make sure that IOExceptions are always converted to MessagingException. + */ + public void testUnreadMessageCountIOException() throws Exception { + runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() { + public void run(MockTransport mockTransport) throws Exception { + mockTransport.expectIOException(); + + mFolder.getUnreadMessageCount(); + } + }); + } + + /** + * Make sure that IOExceptions are always converted to MessagingException. + */ + public void testCopyMessagesIOException() throws Exception { + runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() { + public void run(MockTransport mockTransport) throws Exception { + mockTransport.expectIOException(); + + final Message message = mFolder.createMessage("1"); + final Folder folder = mStore.getFolder("test"); + + mFolder.copyMessages(new Message[] { message }, folder, null); + } + }); + } + + /** + * Make sure that IOExceptions are always converted to MessagingException. + */ + public void testSearchForUidsIOException() throws Exception { + runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() { + public void run(MockTransport mockTransport) throws Exception { + mockTransport.expectIOException(); + + mFolder.getMessage("uid"); + } + }); + } + + /** + * Make sure that IOExceptions are always converted to MessagingException. + */ + public void testExpungeIOException() throws Exception { + runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() { + public void run(MockTransport mockTransport) throws Exception { + mockTransport.expectIOException(); + + mFolder.expunge(); + } + }); + } + + /** + * Make sure that IOExceptions are always converted to MessagingException. + */ + public void testOpenIOException() throws Exception { + runAndExpectMessagingException(new RunAndExpectMessagingExceptionTarget() { + public void run(MockTransport mockTransport) throws Exception { + mockTransport.expectIOException(); + final Folder folder = mStore.getFolder("test"); + folder.open(OpenMode.READ_WRITE, null); + } + }); + } } diff --git a/tests/src/com/android/email/mail/transport/MockTransport.java b/tests/src/com/android/email/mail/transport/MockTransport.java index a54424b2c..81bfb026a 100644 --- a/tests/src/com/android/email/mail/transport/MockTransport.java +++ b/tests/src/com/android/email/mail/transport/MockTransport.java @@ -42,6 +42,8 @@ public class MockTransport implements Transport { private static String LOG_TAG = "MockTransport"; + private static final String SPECIAL_RESPONSE_IOEXCEPTION = "!!!IOEXCEPTION!!!"; + private boolean mSslAllowed = false; private boolean mTlsAllowed = false; @@ -56,8 +58,8 @@ public class MockTransport implements Transport { private static class Transaction { public static final int ACTION_INJECT_TEXT = 0; - public static final int ACTION_SERVER_CLOSE = 1; - public static final int ACTION_CLIENT_CLOSE = 2; + public static final int ACTION_CLIENT_CLOSE = 1; + public static final int ACTION_IO_EXCEPTION = 2; int mAction; String mPattern; @@ -80,10 +82,10 @@ public class MockTransport implements Transport { switch (mAction) { case ACTION_INJECT_TEXT: return mPattern + ": " + Arrays.toString(mResponses); - case ACTION_SERVER_CLOSE: - return "Close the server connection"; case ACTION_CLIENT_CLOSE: return "Expect the client to close"; + case ACTION_IO_EXCEPTION: + return "Expect IOException"; default: return "(Hmm. Unknown action.)"; } @@ -136,9 +138,22 @@ public class MockTransport implements Transport { mPairs.add(new Transaction(Transaction.ACTION_CLIENT_CLOSE)); } - private void sendResponse(String[] responses) { - for (String s : responses) { - mQueuedInput.add(s); + public void expectIOException() { + mPairs.add(new Transaction(Transaction.ACTION_IO_EXCEPTION)); + } + + private void sendResponse(Transaction pair) { + switch (pair.mAction) { + case Transaction.ACTION_INJECT_TEXT: + for (String s : pair.mResponses) { + mQueuedInput.add(s); + } + break; + case Transaction.ACTION_IO_EXCEPTION: + mQueuedInput.add(SPECIAL_RESPONSE_IOEXCEPTION); + break; + default: + Assert.fail("Invalid action for sendResponse: " + pair.mAction); } } @@ -250,18 +265,25 @@ public class MockTransport implements Transport { throw new IOException("Reading from MockTransport with closed input"); } // if there's nothing to read, see if we can find a null-pattern response - if (0 == mQueuedInput.size()) { - Transaction pair = mPairs.size() > 0 ? mPairs.get(0) : null; - if (pair != null && pair.mPattern == null) { + if ((mQueuedInput.size() == 0) && (mPairs.size() > 0)) { + Transaction pair = mPairs.get(0); + if (pair.mPattern == null) { mPairs.remove(0); - sendResponse(pair.mResponses); + sendResponse(pair); } } - SmtpSenderUnitTests.assertTrue("Underflow reading from MockTransport", 0 != mQueuedInput.size()); + if (mQueuedInput.size() == 0) { + // MailTransport returns "" at EOS. + Log.w(LOG_TAG, "Underflow reading from MockTransport"); + return ""; + } String line = mQueuedInput.remove(0); if (DEBUG_LOG_STREAMS) { Log.d(LOG_TAG, "<<< " + line); } + if (SPECIAL_RESPONSE_IOEXCEPTION.equals(line)) { + throw new IOException("Expected IOException."); + } return line; } @@ -292,7 +314,7 @@ public class MockTransport implements Transport { * * Logs the written text if DEBUG_LOG_STREAMS is true. */ - public void writeLine(String s, String sensitiveReplacement) /* throws IOException */ { + public void writeLine(String s, String sensitiveReplacement) throws IOException { if (DEBUG_LOG_STREAMS) { Log.d(LOG_TAG, ">>> " + s); } @@ -300,11 +322,14 @@ public class MockTransport implements Transport { SmtpSenderUnitTests.assertTrue("Overflow writing to MockTransport: Getting " + s, 0 != mPairs.size()); Transaction pair = mPairs.remove(0); + if (pair.mAction == Transaction.ACTION_IO_EXCEPTION) { + throw new IOException("Expected IOException."); + } SmtpSenderUnitTests.assertTrue("Unexpected string written to MockTransport: Actual=" + s + " Expected=" + pair.mPattern, pair.mPattern != null && s.matches(pair.mPattern)); if (pair.mResponses != null) { - sendResponse(pair.mResponses); + sendResponse(pair); } } @@ -354,7 +379,7 @@ public class MockTransport implements Transport { StringBuilder sb = new StringBuilder(); @Override - public void write(int oneByte) { + public void write(int oneByte) throws IOException { // CR or CRLF will immediately dump previous line (w/o CRLF) if (oneByte == '\r') { writeLine(sb.toString(), null);