From 651642faebb48fbac95395c297254fe0afe298e6 Mon Sep 17 00:00:00 2001 From: Daniel Applebaum Date: Tue, 5 May 2009 00:42:05 +0000 Subject: [PATCH] Automatically re-synchronize the IMAP protocol. Previously, losses of synchronization could happen if an Exception was thrown while parsing an untagged response without using executeSimpleCommand. For instance, while doing a fetch. Once synchronization was lost, later commands would fail in surprising ways. One manifestation of such failures was spontaneously emptying of folders when search results were not returned properly. The new code makes sure to only accept OK responses with the tag of the command, and discards the untagged responses from previous command. --- .../android/email/MessagingController.java | 8 ++-- .../android/email/mail/store/ImapStore.java | 45 +++++++++++++++++-- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/com/android/email/MessagingController.java b/src/com/android/email/MessagingController.java index 03016e2ca..6fe4b0291 100644 --- a/src/com/android/email/MessagingController.java +++ b/src/com/android/email/MessagingController.java @@ -753,15 +753,12 @@ public class MessagingController implements Runnable { for (Message thisMess : remoteMessageArray) { remoteMessages.add(thisMess); + remoteUidMap.put(thisMess.getUid(), thisMess); } if (Config.LOGV) { - Log.v(Email.LOG_TAG, "SYNC: Got messages for folder " + folder); + Log.v(Email.LOG_TAG, "SYNC: Got " + remoteUidMap.size() + " messages for folder " + folder); } - for (Message message : remoteMessages) { - remoteUidMap.put(message.getUid(), message); - } - /* * Get a list of the messages that are in the remote list but not on the * local store, or messages that are in the local store but failed to download @@ -923,6 +920,7 @@ s * critical data as fast as possible, and then we'll fill in the de !localMessage.isSet(Flag.DELETED)) { localMessage.setFlag(Flag.X_DESTROYED, true); + // Log.d(Email.LOG_TAG, "Destroying message " + localMessage.getUid() + " which isn't in the most recent group on server"); for (MessagingListener l : getListeners()) { l.synchronizeMailboxRemovedMessage(account, folder, localMessage); } diff --git a/src/com/android/email/mail/store/ImapStore.java b/src/com/android/email/mail/store/ImapStore.java index f936a0e07..8620f71b4 100644 --- a/src/com/android/email/mail/store/ImapStore.java +++ b/src/com/android/email/mail/store/ImapStore.java @@ -384,10 +384,13 @@ public class ImapStore extends Store { mPathDelimeter = nameResponses.get(0).getString(2); } } + + // executeSimpleCommand("CLOSE"); + String command = String.format("SELECT \"%s\"", encodeFolderName(getPrefixedName())); - mMessageCount = -1; + //mMessageCount = -1; List responses = executeSimpleCommand(command); @@ -429,6 +432,11 @@ public class ImapStore extends Store { } public void close(boolean expunge) throws MessagingException { + if (mMessageCount != -1) + { + // close(); + mMessageCount = -1; + } if (!isOpen()) { return; } @@ -436,7 +444,6 @@ public class ImapStore extends Store { { expunge(); } - mMessageCount = -1; synchronized (this) { releaseConnection(mConnection); mConnection = null; @@ -584,6 +591,7 @@ public class ImapStore extends Store { List responses = executeSimpleCommand(String.format("UID SEARCH UID %S", uid)); for (ImapResponse response : responses) { + // Log.d(Email.LOG_TAG, "Got search response: " + response.get(0) + ", size " + response.size()); if (response.mTag == null && response.get(0).equals("SEARCH")) { for (int i = 1, count = response.size(); i < count; i++) { if (uid.equals(response.get(i))) { @@ -614,15 +622,24 @@ public class ImapStore extends Store { checkOpen(); ArrayList messages = new ArrayList(); try { + boolean gotSearchValues = false; ArrayList uids = new ArrayList(); List responses = executeSimpleCommand(String.format("UID SEARCH %d:%d NOT DELETED", start, end)); for (ImapResponse response : responses) { + // Log.d(Email.LOG_TAG, "Got search response: " + response.get(0) + ", size " + response.size()); if (response.get(0).equals("SEARCH")) { + gotSearchValues = true; for (int i = 1, count = response.size(); i < count; i++) { + // Log.d(Email.LOG_TAG, "Got search response UID: " + response.getString(i)); + uids.add(Integer.parseInt(response.getString(i))); } } } + if (gotSearchValues == false) + { + throw new MessagingException("Did not get proper search response"); + } // Sort the uids in numerically ascending order Collections.sort(uids); for (int i = 0, count = uids.size(); i < count; i++) { @@ -868,7 +885,7 @@ public class ImapStore extends Store { private void handleUntaggedResponse(ImapResponse response) { if (response.mTag == null && response.size() > 1 && response.get(1).equals("EXISTS")) { mMessageCount = response.getNumber(0); - Log.i(Email.LOG_TAG, "Got untagged EXISTS with value " + mMessageCount); + //Log.i(Email.LOG_TAG, "Got untagged EXISTS with value " + mMessageCount); } } @@ -1137,6 +1154,15 @@ public class ImapStore extends Store { return null; } + private void close() throws MessagingException { + checkOpen(); + try { + executeSimpleCommand("CLOSE"); + } catch (IOException ioe) { + + } + } + private String combineFlags(Flag[] flags) { ArrayList flagNames = new ArrayList(); @@ -1467,9 +1493,13 @@ public class ImapStore extends Store { throws IOException, ImapException, MessagingException { if (Config.LOGV) { - Log.v(Email.LOG_TAG, "Sending IMAP command " + command); + Log.v(Email.LOG_TAG, "Sending IMAP command " + command + " on connection " + this.hashCode()); } String tag = sendCommand(command, sensitive); + if (Config.LOGV) + { + Log.v(Email.LOG_TAG, "Sent IMAP command " + command + " with tag " + tag); + } ArrayList responses = new ArrayList(); ImapResponse response; do { @@ -1478,6 +1508,13 @@ public class ImapStore extends Store { { Log.v(Email.LOG_TAG, "Got IMAP response " + response); } + if (response.mTag != null && response.mTag.equals(tag) == false) + { + Log.w(Email.LOG_TAG, "Got tag response from previous command " + response); + responses.clear(); + response.mTag = null; + continue; + } responses.add(response); } while (response.mTag == null); if (response.size() < 1 || !response.get(0).equals("OK")) {