Eliminate duplication in Yahoo! sent mailbox

* Yahoo! is not supporting search by UID so I can't identify the new
  the UID after I upload.  This inability to correlate the local and
  remote messages means that we wind up syncing the same message back
  down, in a loop, which spawns more messages.
* Yahoo! has partial support for UIDPLUS, and reports the new UID when
  I append (upload) messages.
* Modify IMAP parser to parse response lists
* When APPENDUID is reported, use it (and skip the search)
* Modify the few other existing users of response lists to use the
  parsed versions instead.  Provided a couple of lightweight utilities
  to make it easier to work with ImapList.
* Unit tests for most of it.
* Optimization: share a static date/time parser for all IMAP connections

Bug: 2448220
Change-Id: Ic10fc1a195ccf4671a498188cc8b17848c8d9df7
This commit is contained in:
Andrew Stadler 2010-03-18 10:11:08 -07:00
parent 0d91d886a9
commit c4fcd852ba
4 changed files with 174 additions and 39 deletions

View File

@ -41,7 +41,7 @@ public class ImapResponseParser {
// mDateTimeFormat is used only for parsing IMAP's FETCH ENVELOPE command, in which
// en_US-like date format is used like "01-Jan-2009 11:20:39 -0800", so this should be
// handled by Locale.US
private final SimpleDateFormat mDateTimeFormat =
private final static SimpleDateFormat DATE_TIME_FORMAT =
new SimpleDateFormat("dd-MMM-yyyy HH:mm:ss Z", Locale.US);
private final PeekableInputStream mIn;
private InputStream mActiveLiteral;
@ -163,7 +163,7 @@ public class ImapResponseParser {
public Object readToken() throws IOException {
while (true) {
Object token = parseToken();
if (token == null || !token.equals(")")) {
if (token == null || !(token.equals(")") || token.equals("]"))) {
return token;
}
}
@ -178,10 +178,15 @@ public class ImapResponseParser {
while (true) {
int ch = mIn.peek();
if (ch == '(') {
return parseList();
return parseList('(', ")");
} else if (ch == ')') {
expect(')');
return ")";
} else if (ch == '[') {
return parseList('[', "]");
} else if (ch == ']') {
expect(']');
return "]";
} else if (ch == '"') {
return parseQuoted();
} else if (ch == '{') {
@ -220,8 +225,14 @@ public class ImapResponseParser {
return tag;
}
private ImapList parseList() throws IOException {
expect('(');
/**
* @param opener The char that the list opens with
* @param closer The char that ends the list
* @return a list object containing the elements of the list
* @throws IOException
*/
private ImapList parseList(char opener, String closer) throws IOException {
expect(opener);
ImapList list = new ImapList();
Object token;
while (true) {
@ -231,7 +242,7 @@ public class ImapResponseParser {
} else if (token instanceof InputStream) {
list.add(token);
break;
} else if (token.equals(")")) {
} else if (token.equals(closer)) {
break;
} else {
list.add(token);
@ -251,9 +262,11 @@ public class ImapResponseParser {
}
throw new IOException("parseAtom(): end of stream reached");
} else if (ch == '(' || ch == ')' || ch == '{' || ch == ' ' ||
// docs claim that flags are \ atom but atom isn't supposed to
// ']' is not part of atom (it's in resp-specials)
ch == ']' ||
// docs claim that flags are \ atom but atom isn't supposed to
// contain
// * and some falgs contain *
// * and some flags contain *
// ch == '%' || ch == '*' ||
ch == '%' ||
// TODO probably should not allow \ and should recognize
@ -335,10 +348,31 @@ public class ImapResponseParser {
return (ImapList)get(index);
}
/** Safe version of getList() */
public ImapList getListOrNull(int index) {
Object list = get(index);
if (list instanceof ImapList) {
return (ImapList) list;
} else {
return null;
}
}
public String getString(int index) {
return (String)get(index);
}
/** Safe version of getString() */
public String getStringOrNull(int index) {
if (index < size()) {
Object string = get(index);
if (string instanceof String) {
return (String) string;
}
}
return null;
}
public InputStream getLiteral(int index) {
return (InputStream)get(index);
}
@ -349,7 +383,7 @@ public class ImapResponseParser {
public Date getDate(int index) throws MessagingException {
try {
return mDateTimeFormat.parse(getString(index));
return DATE_TIME_FORMAT.parse(getString(index));
} catch (ParseException pe) {
throw new MessagingException("Unable to parse IMAP datetime", pe);
}
@ -386,7 +420,7 @@ public class ImapResponseParser {
if (value == null) {
return null;
}
return mDateTimeFormat.parse(value);
return DATE_TIME_FORMAT.parse(value);
} catch (ParseException pe) {
throw new MessagingException("Unable to parse IMAP datetime", pe);
}
@ -447,17 +481,25 @@ public class ImapResponseParser {
return true;
}
// Convert * [ALERT] blah blah blah into "blah blah blah"
public String getAlertText() {
if (size() > 1 && "[ALERT]".equals(getString(1))) {
StringBuffer sb = new StringBuffer();
for (int i = 2, count = size(); i < count; i++) {
sb.append(get(i).toString());
sb.append(' ');
if (size() > 1) {
ImapList alertList = this.getListOrNull(1);
if (alertList != null) {
String responseCode = alertList.getStringOrNull(0);
if ("ALERT".equalsIgnoreCase(responseCode)) {
StringBuffer sb = new StringBuffer();
for (int i = 2, count = size(); i < count; i++) {
if (i > 2) {
sb.append(' ');
}
sb.append(get(i).toString());
}
return sb.toString();
}
}
return sb.toString();
} else {
return null;
}
return null;
}
public String toString() {

View File

@ -518,13 +518,15 @@ public class ImapStore extends Store {
for (ImapResponse response : responses) {
if (response.mTag == null && response.get(1).equals("EXISTS")) {
mMessageCount = response.getNumber(0);
}
else if (response.mTag != null && response.size() >= 2) {
if ("[READ-ONLY]".equalsIgnoreCase(response.getString(1))) {
mMode = OpenMode.READ_ONLY;
}
else if ("[READ-WRITE]".equalsIgnoreCase(response.getString(1))) {
mMode = OpenMode.READ_WRITE;
} else if (response.mTag != null) {
ImapList responseList = response.getListOrNull(1);
if (responseList != null) {
String atom = responseList.getStringOrNull(0);
if ("READ-ONLY".equalsIgnoreCase(atom)) {
mMode = OpenMode.READ_ONLY;
} else if ("READ-WRITE".equalsIgnoreCase(atom)) {
mMode = OpenMode.READ_WRITE;
}
}
}
}
@ -1204,12 +1206,23 @@ public class ImapStore extends Store {
handleUntaggedResponse(response);
}
while (response.more());
} while(response.mTag == null);
} while (response.mTag == null);
/*
* Try to recover the UID of the message from an APPENDUID response.
* e.g. 11 OK [APPENDUID 2 238268] APPEND completed
*/
ImapList appendList = response.getListOrNull(1);
if (appendList != null && appendList.size() == 3 &&
"APPENDUID".equalsIgnoreCase(appendList.getString(0))) {
String serverUid = appendList.getString(2);
message.setUid(serverUid);
continue;
}
/*
* Try to find the UID of the message we just appended using the
* Message-ID header. If there are more than one response, take the
* last one, as it's most likely he newest (the one we just uploaded).
* last one, as it's most likely the newest (the one we just uploaded).
*/
String messageId = message.getMessageId();
if (messageId == null || messageId.length() == 0) {

View File

@ -25,6 +25,7 @@ import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;
import java.io.ByteArrayInputStream;
import java.io.IOException;
/**
* This is a series of unit tests for the ImapStore class. These tests must be locally
@ -52,6 +53,8 @@ public class ImapResponseParserUnitTests extends AndroidTestCase {
assertNull("Line 1 tag", line1.mTag);
assertTrue("Line 1 completed", line1.completed());
assertEquals("Line 1 count", 3, line1.size());
Object line1list = line1.get(2);
assertEquals("Line 1 list count", 2, ((ImapList)line1list).size());
ImapResponse line2 = parser.readResponse();
assertEquals("Line 2 tag", "100", line2.mTag);
@ -84,4 +87,48 @@ public class ImapResponseParserUnitTests extends AndroidTestCase {
assertTrue("Line 5 completed", line5.completed());
assertEquals("Line 5 count", 3, line5.size());
}
/**
* Test for parsing expansion resp-text in OK or related responses
*/
public void testParseResponseText() throws Exception {
ByteArrayInputStream is = new ByteArrayInputStream(
("101 OK STATUS completed\r\n"
+ "102 OK [APPENDUID 2 238257] APPEND completed\r\n")
.getBytes());
ImapResponseParser parser = new ImapResponseParser(is, new DiscourseLogger(4));
ImapResponse line1 = parser.readResponse();
assertEquals("101", line1.mTag);
assertTrue(line1.completed());
assertEquals(3, line1.size()); // "OK STATUS COMPLETED"
ImapResponse line2 = parser.readResponse();
assertEquals("102", line2.mTag);
assertTrue(line2.completed());
assertEquals(4, line2.size()); // "OK [APPENDUID 2 238257] APPEND completed"
Object responseList = line2.get(1);
assertEquals(3, ((ImapList)responseList).size());
}
/**
* Test special parser of [ALERT] responses
*/
public void testAlertText() throws IOException {
ByteArrayInputStream is = new ByteArrayInputStream(
("* OK [AlErT] system going down\r\n"
+ "* OK [ALERT]\r\n"
+ "* OK [SOME-OTHER-TAG]\r\n")
.getBytes());
ImapResponseParser parser = new ImapResponseParser(is, new DiscourseLogger(4));
ImapResponse line1 = parser.readResponse();
assertEquals("system going down", line1.getAlertText());
ImapResponse line2 = parser.readResponse();
assertEquals("", line2.getAlertText());
ImapResponse line3 = parser.readResponse();
assertNull(line3.getAlertText());
}
}

View File

@ -223,7 +223,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
"* ID (\"name\" \"Cyrus\" \"version\" \"1.5\"" +
" \"os\" \"sunos\" \"os-version\" \"5.5\"" +
" \"support-url\" \"mailto:cyrus-bugs+@andrew.cmu.edu\")",
"OK"});
"OK"}, "READ-WRITE");
mFolder.open(OpenMode.READ_WRITE, null);
}
@ -236,7 +236,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
// try to open it
setupOpenFolder(mockTransport, new String[] {
"* ID NIL",
"OK [ID] bad-char-%"});
"OK [ID] bad-char-%"}, "READ-WRITE");
mFolder.open(OpenMode.READ_WRITE, null);
}
@ -248,7 +248,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
// try to open it
setupOpenFolder(mockTransport, new String[] {
"BAD unknown command bad-char-%"});
"BAD unknown command bad-char-%"}, "READ-WRITE");
mFolder.open(OpenMode.READ_WRITE, null);
}
@ -349,30 +349,41 @@ public class ImapStoreUnitTests extends AndroidTestCase {
mStore.setTransport(mockTransport);
return mockTransport;
}
/**
* Helper which stuffs the mock with enough strings to satisfy a call to ImapFolder.open()
*
*
* @param mockTransport the mock transport we're using
*/
private void setupOpenFolder(MockTransport mockTransport) {
setupOpenFolder(mockTransport, new String[] {
"* ID NIL", "OK"});
setupOpenFolder(mockTransport, "READ-WRITE");
}
/**
* Helper which stuffs the mock with enough strings to satisfy a call to ImapFolder.open()
*
* @param mockTransport the mock transport we're using
*/
private void setupOpenFolder(MockTransport mockTransport, String readWriteMode) {
setupOpenFolder(mockTransport, new String[] {
"* ID NIL", "OK"}, readWriteMode);
}
/**
* Helper which stuffs the mock with enough strings to satisfy a call to ImapFolder.open()
* Also allows setting a custom IMAP ID.
*
* Also sets mNextTag, an int, which is useful if there are additional commands to inject.
*
*
* @param mockTransport the mock transport we're using
* @param imapIdResponse the expected series of responses to the IMAP ID command. Non-final
* lines should be tagged with *. The final response should be untagged (the correct
* tag will be added at runtime).
* @param "READ-WRITE" or "READ-ONLY"
* @return the next tag# to use
*/
private void setupOpenFolder(MockTransport mockTransport, String[] imapIdResponse) {
private void setupOpenFolder(MockTransport mockTransport, String[] imapIdResponse,
String readWriteMode) {
// Fix the tag # of the ID response
String last = imapIdResponse[imapIdResponse.length-1];
last = "2 " + last;
@ -392,7 +403,7 @@ public class ImapStoreUnitTests extends AndroidTestCase {
"* 0 RECENT",
"* OK [UNSEEN 0]",
"* OK [UIDNEXT 1]",
"4 OK [READ-WRITE] INBOX selected. (Success)"});
"4 OK [" + readWriteMode + "] INBOX selected. (Success)"});
mNextTag = 5;
}
@ -407,7 +418,29 @@ public class ImapStoreUnitTests extends AndroidTestCase {
if (advance) ++mNextTag;
return Integer.toString(mNextTag);
}
/**
* Test that servers reporting READ-WRITE mode are parsed properly
* Note: the READ_WRITE mode passed to folder.open() does not affect the test
*/
public void testReadWrite() throws MessagingException {
MockTransport mock = openAndInjectMockTransport();
setupOpenFolder(mock, "READ-WRITE");
mFolder.open(OpenMode.READ_WRITE, null);
assertEquals(OpenMode.READ_WRITE, mFolder.getMode());
}
/**
* Test that servers reporting READ-ONLY mode are parsed properly
* Note: the READ_ONLY mode passed to folder.open() does not affect the test
*/
public void testReadOnly() throws MessagingException {
MockTransport mock = openAndInjectMockTransport();
setupOpenFolder(mock, "READ-ONLY");
mFolder.open(OpenMode.READ_ONLY, null);
assertEquals(OpenMode.READ_ONLY, mFolder.getMode());
}
/**
* Test for getUnreadMessageCount with quoted string in the middle of response.
*/