From d31238ca881354938b9d923819da3c63ffb4ac12 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Mon, 21 Mar 2011 16:34:44 -0700 Subject: [PATCH] Add support to query for message IDs If an IMAP server supports the UIDPLUS capability, it can return the new UID as part of the response to the "UID COPY" command. However, if the server does not support UIDPLUS, we perform a SEARCH to try to determine the new message UID. This is the second of a couple modifications. bug 4092301 Change-Id: I1f548b63becfec8733cb8ba9a3fe6ff4be6fdd83 --- .../android/email/MessagingController.java | 2 + .../android/email/mail/store/ImapStore.java | 97 ++++++++----- .../email/mail/store/ImapStoreUnitTests.java | 136 ++++++++++++++++-- 3 files changed, 184 insertions(+), 51 deletions(-) diff --git a/src/com/android/email/MessagingController.java b/src/com/android/email/MessagingController.java index 86b63aaa0..f97f36cb7 100644 --- a/src/com/android/email/MessagingController.java +++ b/src/com/android/email/MessagingController.java @@ -1479,6 +1479,8 @@ public class MessagingController implements Runnable { if (!remoteFolder.exists()) { return; } + // We may need the message id to search for the message in the destination folder + remoteMessage.setMessageId(newMessage.mMessageId); // Copy the message to its new folder remoteFolder.copyMessages(messages, toFolder, new MessageUpdateCallbacks() { @Override diff --git a/src/com/android/email/mail/store/ImapStore.java b/src/com/android/email/mail/store/ImapStore.java index 2596d5b7f..e7e64de6d 100644 --- a/src/com/android/email/mail/store/ImapStore.java +++ b/src/com/android/email/mail/store/ImapStore.java @@ -608,39 +608,7 @@ public class ImapStore extends Store { // * OK [UIDNEXT 57576] Predicted next UID // 2 OK [READ-WRITE] Select completed. try { - List responses = mConnection.executeSimpleCommand( - String.format(ImapConstants.SELECT + " \"%s\"", - encodeFolderName(mName, mStore.mPathPrefix))); - /* - * If the command succeeds we expect the folder has been opened read-write - * unless we are notified otherwise in the responses. - */ - mMode = OpenMode.READ_WRITE; - - int messageCount = -1; - for (ImapResponse response : responses) { - if (response.isDataResponse(1, ImapConstants.EXISTS)) { - messageCount = response.getStringOrEmpty(0).getNumberOrZero(); - - } else if (response.isOk()) { - final ImapString responseCode = response.getResponseCodeOrEmpty(); - if (responseCode.is(ImapConstants.READ_ONLY)) { - mMode = OpenMode.READ_ONLY; - } else if (responseCode.is(ImapConstants.READ_WRITE)) { - mMode = OpenMode.READ_WRITE; - } - } else if (response.isTagged()) { // Not OK - throw new MessagingException("Can't open mailbox: " - + response.getStatusResponseTextOrEmpty()); - } - } - - if (messageCount == -1) { - throw new MessagingException("Did not find message count during select"); - } - mMessageCount = messageCount; - mExists = true; - + doSelect(); } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { @@ -772,12 +740,9 @@ public class ImapStore extends Store { String.format(ImapConstants.UID_COPY + " %s \"%s\"", joinMessageUids(messages), encodeFolderName(folder.getName(), mStore.mPathPrefix))); - if (!mConnection.isCapable(ImapConnection.CAPABILITY_UIDPLUS)) { - // TODO Implement alternate way to fetch UIDs (e.g. perform a query) - return; - } // Build a message map for faster UID matching HashMap messageMap = new HashMap(); + boolean handledUidPlus = false; for (Message m : messages) { messageMap.put(m.getUid(), m); } @@ -800,6 +765,7 @@ public class ImapStore extends Store { ImapList copyResponse = response.getListOrEmpty(1); String responseCode = copyResponse.getStringOrEmpty(0).getString(); if (ImapConstants.COPYUID.equals(responseCode)) { + handledUidPlus = true; String origIdSet = copyResponse.getStringOrEmpty(2).getString(); String newIdSet = copyResponse.getStringOrEmpty(3).getString(); String[] origIdArray = ImapUtility.getImapSequenceValues(origIdSet); @@ -818,6 +784,29 @@ public class ImapStore extends Store { } } } + // If the server doesn't support UIDPLUS, try a different way to get the new UID(s) + if (callbacks != null && !handledUidPlus) { + ImapFolder newFolder = (ImapFolder)folder; + try { + // Temporarily select the destination folder + newFolder.open(OpenMode.READ_WRITE, null); + // Do the search(es) ... + for (Message m : messages) { + String searchString = "HEADER Message-Id \"" + m.getMessageId() + "\""; + String[] newIdArray = newFolder.searchForUids(searchString); + if (newIdArray.length == 1) { + callbacks.onMessageUidChange(m, newIdArray[0]); + } + } + } catch (MessagingException e) { + // Log, but, don't abort; failures here don't need to be propagated + Log.d(Logging.LOG_TAG, "Failed to find message", e); + } finally { + newFolder.close(false); + } + // Re-select the original folder + doSelect(); + } } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } finally { @@ -1478,6 +1467,40 @@ public class ImapStore extends Store { } } + /** + * Selects the folder for use. Before performing any operations on this folder, it + * must be selected. + */ + private void doSelect() throws IOException, MessagingException { + List responses = mConnection.executeSimpleCommand( + String.format(ImapConstants.SELECT + " \"%s\"", + encodeFolderName(mName, mStore.mPathPrefix))); + + // Assume the folder is opened read-write; unless we are notified otherwise + mMode = OpenMode.READ_WRITE; + int messageCount = -1; + for (ImapResponse response : responses) { + if (response.isDataResponse(1, ImapConstants.EXISTS)) { + messageCount = response.getStringOrEmpty(0).getNumberOrZero(); + } else if (response.isOk()) { + final ImapString responseCode = response.getResponseCodeOrEmpty(); + if (responseCode.is(ImapConstants.READ_ONLY)) { + mMode = OpenMode.READ_ONLY; + } else if (responseCode.is(ImapConstants.READ_WRITE)) { + mMode = OpenMode.READ_WRITE; + } + } else if (response.isTagged()) { // Not OK + throw new MessagingException("Can't open mailbox: " + + response.getStatusResponseTextOrEmpty()); + } + } + if (messageCount == -1) { + throw new MessagingException("Did not find message count during select"); + } + mMessageCount = messageCount; + mExists = true; + } + private void checkOpen() throws MessagingException { if (!isOpen()) { throw new MessagingException("Folder " + mName + " is not open."); diff --git a/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java b/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java index 136ea4b68..7d0574004 100644 --- a/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java +++ b/tests/src/com/android/email/mail/store/ImapStoreUnitTests.java @@ -428,7 +428,7 @@ public class ImapStoreUnitTests extends AndroidTestCase { mockTransport.expect(getNextTag(false) + " LOGIN user \"password\"", getNextTag(true) + " " + "oK user authenticated (Success)"); // SELECT - expectSelect(mockTransport, "rEAD-wRITE"); + expectSelect(mockTransport, FOLDER_ENCODED, "rEAD-wRITE"); // Now open the folder. Although the server indicates ID in the capabilities, // we are not expecting the store to send the ID command (to this particular server). @@ -542,7 +542,7 @@ public class ImapStoreUnitTests extends AndroidTestCase { private void setupOpenFolder(MockTransport mockTransport, String[] imapIdResponse, String readWriteMode, boolean withUidPlus) { expectLogin(mockTransport, imapIdResponse, withUidPlus); - expectSelect(mockTransport, readWriteMode); + expectSelect(mockTransport, FOLDER_ENCODED, readWriteMode); } /** @@ -550,9 +550,9 @@ public class ImapStoreUnitTests extends AndroidTestCase { * @param mockTransport the mock transport we're using * @param readWriteMode "READ-WRITE" or "READ-ONLY" */ - private void expectSelect(MockTransport mockTransport, String readWriteMode) { + private void expectSelect(MockTransport mockTransport, String folder, String readWriteMode) { mockTransport.expect( - getNextTag(false) + " SELECT \"" + FOLDER_ENCODED + "\"", new String[] { + getNextTag(false) + " SELECT \"" + folder + "\"", new String[] { "* fLAGS (\\Answered \\Flagged \\Draft \\Deleted \\Seen)", "* oK [pERMANENTFLAGS (\\Answered \\Flagged \\Draft \\Deleted \\Seen \\*)]", "* 0 eXISTS", @@ -560,7 +560,7 @@ public class ImapStoreUnitTests extends AndroidTestCase { "* OK [uNSEEN 0]", "* OK [uIDNEXT 1]", getNextTag(true) + " oK [" + readWriteMode + "] " + - FOLDER_ENCODED + " selected. (Success)"}); + folder + " selected. (Success)"}); } private void expectLogin(MockTransport mockTransport) { @@ -1470,17 +1470,31 @@ public class ImapStoreUnitTests extends AndroidTestCase { mFolder.open(OpenMode.READ_WRITE, null); mCopyToFolder = mStore.getFolder("\u65E5\u672C\u8A9E"); - mCopyMessages = new Message[] { - mFolder.createMessage("11"), - mFolder.createMessage("12"), - }; + Message m1 = mFolder.createMessage("11"); + m1.setMessageId("<4D8978AE.0000005D@m58.foo.com>"); + Message m2 = mFolder.createMessage("12"); + m2.setMessageId("<549373104MSOSI1:145OSIMS@bar.com>"); + mCopyMessages = new Message[] { m1, m2 }; } + /** + * Returns the pattern for the IMAP request to copy messages. + */ private String getCopyMessagesPattern() { return getNextTag(false) + " UID COPY 11\\,12 \\\"&ZeVnLIqe-\\\""; } - private static class CopyMessagesCallback implements Folder.MessageUpdateCallbacks { + /** + * Returns the pattern for the IMAP request to search for messages based on Message-Id. + */ + private String getSearchMessagesPattern(String messageId) { + return getNextTag(false) + " UID SEARCH HEADER Message-Id \"" + messageId + "\""; + } + + /** + * Counts the number of times the callback methods are invoked. + */ + private static class MessageUpdateCallbackCounter implements Folder.MessageUpdateCallbacks { int messageNotFoundCalled; int messageUidChangeCalled; @@ -1504,7 +1518,7 @@ public class ImapStoreUnitTests extends AndroidTestCase { getNextTag(true) + " oK [COPYUID 777 11,12 45,46] UID COPY completed" }); - CopyMessagesCallback cb = new CopyMessagesCallback(); + MessageUpdateCallbackCounter cb = new MessageUpdateCallbackCounter(); mFolder.copyMessages(mCopyMessages, mCopyToFolder, cb); assertEquals(0, cb.messageNotFoundCalled); @@ -1520,7 +1534,7 @@ public class ImapStoreUnitTests extends AndroidTestCase { getNextTag(true) + " oK [COPYUID 777 11,12 45,46] UID COPY completed" }); - CopyMessagesCallback cb = new CopyMessagesCallback(); + MessageUpdateCallbackCounter cb = new MessageUpdateCallbackCounter(); mFolder.copyMessages(mCopyMessages, mCopyToFolder, cb); assertEquals(0, cb.messageNotFoundCalled); @@ -1566,8 +1580,8 @@ public class ImapStoreUnitTests extends AndroidTestCase { setupCopyMessages(false); mCopyMock.expect(getCopyMessagesPattern(), new String[] { - getNextTag(true) + " BaD copy completed" - }); + getNextTag(true) + " BaD copy completed" + }); mFolder.copyMessages(mCopyMessages, mCopyToFolder, null); @@ -1576,6 +1590,100 @@ public class ImapStoreUnitTests extends AndroidTestCase { } } + // Golden case; successful copy getting UIDs via search + public void testCopyMessages6() throws Exception { + setupCopyMessages(false); + mCopyMock.expect(getCopyMessagesPattern(), + new String[] { + getNextTag(true) + " oK UID COPY completed", + }); + // New connection, so, we need to login again + expectLogin(mCopyMock, new String[] {"* iD nIL", "oK"}, false); + // Select destination folder + expectSelect(mCopyMock, "&ZeVnLIqe-", "rEAD-wRITE"); + // Perform searches + mCopyMock.expect(getSearchMessagesPattern("<4D8978AE.0000005D@m58.foo.com>"), + new String[] { + "* SeArCh 777", + getNextTag(true) + " oK UID SEARCH completed (1 msgs in 3.14159 secs)", + }); + mCopyMock.expect(getSearchMessagesPattern("<549373104MSOSI1:145OSIMS@bar.com>"), + new String[] { + "* sEaRcH 1818", + getNextTag(true) + " oK UID SEARCH completed (1 msgs in 2.71828 secs)", + }); + // Select the original folder + expectSelect(mCopyMock, FOLDER_ENCODED, "rEAD-wRITE"); + + MessageUpdateCallbackCounter cb = new MessageUpdateCallbackCounter(); + mFolder.copyMessages(mCopyMessages, mCopyToFolder, cb); + + assertEquals(0, cb.messageNotFoundCalled); + assertEquals(2, cb.messageUidChangeCalled); + } + + // Degenerate case; searches turn up nothing + public void testCopyMessages7() throws Exception { + setupCopyMessages(false); + mCopyMock.expect(getCopyMessagesPattern(), + new String[] { + getNextTag(true) + " oK UID COPY completed", + }); + // New connection, so, we need to login again + expectLogin(mCopyMock, new String[] {"* iD nIL", "oK"}, false); + // Select destination folder + expectSelect(mCopyMock, "&ZeVnLIqe-", "rEAD-wRITE"); + // Perform searches + mCopyMock.expect(getSearchMessagesPattern("<4D8978AE.0000005D@m58.foo.com>"), + new String[] { + "* SeArCh", + getNextTag(true) + " oK UID SEARCH completed (0 msgs in 6.02214 secs)", + }); + mCopyMock.expect(getSearchMessagesPattern("<549373104MSOSI1:145OSIMS@bar.com>"), + new String[] { + "* sEaRcH", + getNextTag(true) + " oK UID SEARCH completed (0 msgs in 2.99792 secs)", + }); + // Select the original folder + expectSelect(mCopyMock, FOLDER_ENCODED, "rEAD-wRITE"); + + MessageUpdateCallbackCounter cb = new MessageUpdateCallbackCounter(); + mFolder.copyMessages(mCopyMessages, mCopyToFolder, cb); + + assertEquals(0, cb.messageNotFoundCalled); + assertEquals(0, cb.messageUidChangeCalled); + } + + // Degenerate case; search causes an exception; must be eaten + public void testCopyMessages8() throws Exception { + setupCopyMessages(false); + mCopyMock.expect(getCopyMessagesPattern(), + new String[] { + getNextTag(true) + " oK UID COPY completed", + }); + // New connection, so, we need to login again + expectLogin(mCopyMock, new String[] {"* iD nIL", "oK"}, false); + // Select destination folder + expectSelect(mCopyMock, "&ZeVnLIqe-", "rEAD-wRITE"); + // Perform searches + mCopyMock.expect(getSearchMessagesPattern("<4D8978AE.0000005D@m58.foo.com>"), + new String[] { + getNextTag(true) + " BaD search failed" + }); + mCopyMock.expect(getSearchMessagesPattern("<549373104MSOSI1:145OSIMS@bar.com>"), + new String[] { + getNextTag(true) + " BaD search failed" + }); + // Select the original folder + expectSelect(mCopyMock, FOLDER_ENCODED, "rEAD-wRITE"); + + MessageUpdateCallbackCounter cb = new MessageUpdateCallbackCounter(); + mFolder.copyMessages(mCopyMessages, mCopyToFolder, cb); + + assertEquals(0, cb.messageNotFoundCalled); + assertEquals(0, cb.messageUidChangeCalled); + } + public void testGetUnreadMessageCount() throws Exception { MockTransport mock = openAndInjectMockTransport(); setupOpenFolder(mock);