Merge "Fix handling IOException in ImapStore"

This commit is contained in:
Makoto Onuki 2010-08-11 14:44:47 -07:00 committed by Android (Google) Code Review
commit 41502f6ea8
3 changed files with 181 additions and 31 deletions

View File

@ -517,10 +517,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) {
@ -533,7 +538,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.
@ -589,7 +594,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
} catch (MessagingException e) {
mExists = false;
@ -613,6 +618,7 @@ public class ImapStore extends Store {
// TODO implement expunge
mMessageCount = -1;
synchronized (this) {
destroyResponses();
mStore.poolConnection(mConnection);
mConnection = null;
}
@ -714,7 +720,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -742,7 +748,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -785,7 +791,7 @@ public class ImapStore extends Store {
return ret.toArray(Utility.EMPTY_STRINGS);
}
} finally {
mConnection.destroyResponses();
destroyResponses();
}
return Utility.EMPTY_STRINGS;
}
@ -848,16 +854,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();
@ -910,7 +916,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);
@ -1004,7 +1010,7 @@ public class ImapStore extends Store {
listener.messageRetrieved(message);
}
} finally {
mConnection.destroyResponses();
destroyResponses();
}
} while (!response.isTagged());
} catch (IOException ioe) {
@ -1298,7 +1304,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -1310,7 +1316,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
return null;
}
@ -1345,7 +1351,7 @@ public class ImapStore extends Store {
} catch (IOException ioe) {
throw ioExceptionHandler(mConnection, ioe);
} finally {
mConnection.destroyResponses();
destroyResponses();
}
}
@ -1357,8 +1363,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);
}

View File

@ -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;
@ -1551,4 +1552,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);
}
});
}
}

View File

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