Fixes Issue 1551

Fixes Issue 1577

Issue 1551: 
Some IMAP servers send untagged EXPUNGEs to IDLEing
clients without ever haven't sent an untagged FETCH.  The untagged
EXPUNGEs are harder to deal with because they don't have a UID.  So,
if the user has elected to have the IDLE connection start with a poll,
we can maintain a map of message sequence numbers to UIDs that we can
use to figure out which message to delete.  To mitigate the risk of
the map falling out of date, we do a UID SEARCH UID before removing
the local copy of the message, just to make sure the message is really
gone from the server.  If we detect an error, do another poll to
resync the map.

Issue 1577:
Restore the removal of notifications for an account when the account's
unread message count goes to 0.

src/com/fsck/k9/MessagingController.java

@@ -929,7 +929,7 @@ public class MessagingController implements Runnable
             LocalStore localStore = account.getLocalStore();
             LocalFolder localFolder = localStore.getFolder(folder);
             localFolder.setVisibleLimit(localFolder.getVisibleLimit() + account.getDisplayCount());
-            synchronizeMailbox(account, folder, listener);
+            synchronizeMailbox(account, folder, listener, null);
         }
         catch (MessagingException me)
         {
@@ -962,14 +962,15 @@ public class MessagingController implements Runnable
      * @param account
      * @param folder
      * @param listener
+     * @param providedRemoteFolder TODO
      */
-    public void synchronizeMailbox(final Account account, final String folder, final MessagingListener listener)
+    public void synchronizeMailbox(final Account account, final String folder, final MessagingListener listener, final Folder providedRemoteFolder)
     {
         putBackground("synchronizeMailbox", listener, new Runnable()
         {
             public void run()
             {
-                synchronizeMailboxSynchronous(account, folder, listener);
+                synchronizeMailboxSynchronous(account, folder, listener, providedRemoteFolder);
             }
         });
     }
@@ -981,8 +982,9 @@ public class MessagingController implements Runnable
      * @param folder
      *
      * TODO Break this method up into smaller chunks.
+     * @param providedRemoteFolder TODO
      */
-    public void synchronizeMailboxSynchronous(final Account account, final String folder, final MessagingListener listener)
+    private void synchronizeMailboxSynchronous(final Account account, final String folder, final MessagingListener listener, Folder providedRemoteFolder)
     {
         Folder remoteFolder = null;
         LocalFolder tLocalFolder = null;
@@ -1052,47 +1054,54 @@ public class MessagingController implements Runnable
                 localUidMap.put(message.getUid(), message);
             }
 
-            if (K9.DEBUG)
+            if (providedRemoteFolder != null)
-                Log.v(K9.LOG_TAG, "SYNC: About to get remote store for " + folder);
-
-            Store remoteStore = account.getRemoteStore();
-
-            if (K9.DEBUG)
-                Log.v(K9.LOG_TAG, "SYNC: About to get remote folder " + folder);
-
-            remoteFolder = remoteStore.getFolder(folder);
-
-            /*
-             * If the folder is a "special" folder we need to see if it exists
-             * on the remote server. It if does not exist we'll try to create it. If we
-             * can't create we'll abort. This will happen on every single Pop3 folder as
-             * designed and on Imap folders during error conditions. This allows us
-             * to treat Pop3 and Imap the same in this code.
-             */
-            if (folder.equals(account.getTrashFolderName()) ||
-                    folder.equals(account.getSentFolderName()) ||
-                    folder.equals(account.getDraftsFolderName()))
             {
-                if (!remoteFolder.exists())
+                if (K9.DEBUG)
-                {
+                    Log.v(K9.LOG_TAG, "SYNC: using providedRemoteFolder " + folder);
-                    if (!remoteFolder.create(FolderType.HOLDS_MESSAGES))
-                    {
-                        for (MessagingListener l : getListeners())
-                        {
-                            l.synchronizeMailboxFinished(account, folder, 0, 0);
-                        }
-                        if (listener != null && getListeners().contains(listener) == false)
-                        {
-                            listener.synchronizeMailboxFinished(account, folder, 0, 0);
-                        }
-                        if (K9.DEBUG)
-                            Log.i(K9.LOG_TAG, "Done synchronizing folder " + folder);
 
-                        return;
+                remoteFolder = providedRemoteFolder;
+            }
+            else
+            {
+                
+                if (K9.DEBUG)
+                    Log.v(K9.LOG_TAG, "SYNC: About to get remote store for " + folder);
+
+                Store remoteStore = account.getRemoteStore();
+                if (K9.DEBUG)
+                    Log.v(K9.LOG_TAG, "SYNC: About to get remote folder " + folder);
+                remoteFolder = remoteStore.getFolder(folder);
+
+                /*
+                 * If the folder is a "special" folder we need to see if it exists
+                 * on the remote server. It if does not exist we'll try to create it. If we
+                 * can't create we'll abort. This will happen on every single Pop3 folder as
+                 * designed and on Imap folders during error conditions. This allows us
+                 * to treat Pop3 and Imap the same in this code.
+                 */
+                if (folder.equals(account.getTrashFolderName()) ||
+                        folder.equals(account.getSentFolderName()) ||
+                        folder.equals(account.getDraftsFolderName()))
+                {
+                    if (!remoteFolder.exists())
+                    {
+                        if (!remoteFolder.create(FolderType.HOLDS_MESSAGES))
+                        {
+                            for (MessagingListener l : getListeners())
+                            {
+                                l.synchronizeMailboxFinished(account, folder, 0, 0);
+                            }
+                            if (listener != null && getListeners().contains(listener) == false)
+                            {
+                                listener.synchronizeMailboxFinished(account, folder, 0, 0);
+                            }
+                            if (K9.DEBUG)
+                                Log.i(K9.LOG_TAG, "Done synchronizing folder " + folder);
+    
+                            return;
+                        }
                     }
                 }
-            }
-
             /*
              * Synchronization process:
             Open the folder
@@ -1115,11 +1124,11 @@ public class MessagingController implements Runnable
             /*
              * Open the remote folder. This pre-loads certain metadata like message count.
              */
-            if (K9.DEBUG)
+                if (K9.DEBUG)
-                Log.v(K9.LOG_TAG, "SYNC: About to open remote folder " + folder);
+                    Log.v(K9.LOG_TAG, "SYNC: About to open remote folder " + folder);
-
-            remoteFolder.open(OpenMode.READ_WRITE);
     
+                remoteFolder.open(OpenMode.READ_WRITE);
+            }
             if (Account.EXPUNGE_ON_POLL.equals(account.getExpungePolicy()))
             {
                 if (K9.DEBUG)
@@ -1290,7 +1299,7 @@ public class MessagingController implements Runnable
         }
         finally
         {
-            if (remoteFolder != null)
+            if (providedRemoteFolder == null && remoteFolder != null)
             {
                 remoteFolder.close();
             }
@@ -1360,7 +1369,11 @@ public class MessagingController implements Runnable
 
         for (Message message : messages)
         {
-            if (isMessageSuppressed(account, folder, message) == false)
+            if (message.isSet(Flag.DELETED))
+            {
+                syncFlagMessages.add(message);
+            }
+            else if (isMessageSuppressed(account, folder, message) == false)
             {
                 Message localMessage = localFolder.getMessage(message.getUid());
 
@@ -1756,16 +1769,23 @@ public class MessagingController implements Runnable
          */
         if (remoteFolder.supportsFetchingFlags())
         {
-
-
             if (K9.DEBUG)
                 Log.d(K9.LOG_TAG, "SYNC: About to sync flags for "
                       + syncFlagMessages.size() + " remote messages for folder " + folder);
 
-
             fp.clear();
             fp.add(FetchProfile.Item.FLAGS);
-            remoteFolder.fetch(syncFlagMessages.toArray(new Message[0]), fp, null);
+            
+            List<Message> undeletedMessages = new LinkedList<Message>();
+            for (Message message : syncFlagMessages)
+            {
+                if (message.isSet(Flag.DELETED) == false)
+                {
+                    undeletedMessages.add(message);
+                }
+            }
+            
+            remoteFolder.fetch(undeletedMessages.toArray(new Message[0]), fp, null);
             for (Message remoteMessage : syncFlagMessages)
             {
                 Message localMessage = localFolder.getMessage(remoteMessage.getUid());
@@ -4243,7 +4263,7 @@ public class MessagingController implements Runnable
                                             }
                                             try
                                             {
-                                                synchronizeMailboxSynchronous(account, folder.getName(), listener);
+                                                synchronizeMailboxSynchronous(account, folder.getName(), listener, null);
                                             }
                                             finally
                                             {
@@ -4286,6 +4306,19 @@ public class MessagingController implements Runnable
                                     if (K9.DEBUG)
                                         Log.v(K9.LOG_TAG, "Clearing notification flag for " + account.getDescription());
                                     account.setRingNotified(false);
+                                    try
+                                    {
+                                        AccountStats stats = account.getStats(context);
+                                        int unreadMessageCount = stats.unreadMessageCount;
+                                        if (unreadMessageCount == 0)
+                                        {
+                                            notifyAccountCancel(context, account);
+                                        }
+                                    }
+                                    catch (MessagingException e)
+                                    {
+                                        Log.e(K9.LOG_TAG, "Unable to getUnreadMessageCount for account: " + account, e);
+                                    }
                                 }
                             }
                                          );
@@ -4429,6 +4462,17 @@ public class MessagingController implements Runnable
     */
     private boolean notifyAccount(Context context, Account account, Message message)
     {
+        int unreadMessageCount = 0;
+        try
+        {
+            AccountStats stats = account.getStats(context);
+            unreadMessageCount = stats.unreadMessageCount;
+        }
+        catch (MessagingException e)
+        {
+            Log.e(K9.LOG_TAG, "Unable to getUnreadMessageCount for account: " + account, e);
+        }
+
         // Do not notify if the user does not have notifications
         // enabled or if the message has been read
         if (!account.isNotifyNewMail() || message.isSet(Flag.SEEN))
@@ -4504,16 +4548,6 @@ public class MessagingController implements Runnable
             messageNotice.append(context.getString(R.string.notification_new_title));
         }
 
-        int unreadMessageCount = 0;
-        try
-        {
-            AccountStats stats = account.getStats(context);
-            unreadMessageCount = stats.unreadMessageCount;
-        }
-        catch (MessagingException e)
-        {
-            Log.e(K9.LOG_TAG, "Unable to getUnreadMessageCount for account: " + account, e);
-        }
         
         NotificationManager notifMgr =
             (NotificationManager)context.getSystemService(Context.NOTIFICATION_SERVICE);

src/com/fsck/k9/MessagingControllerPushReceiver.java

@@ -69,12 +69,15 @@ public class MessagingControllerPushReceiver implements PushReceiver
                                      List<Message> messages)
     {
         controller.messagesArrived(account, folder, messages, true);
-
     }
     public void messagesArrived(Folder folder, List<Message> messages)
     {
         controller.messagesArrived(account, folder, messages, false);
     }
+    public void messagesRemoved(Folder folder, List<Message> messages)
+    {
+        controller.messagesArrived(account, folder, messages, true);
+    }
 
     public void syncFolder(Folder folder)
     {
@@ -96,7 +99,7 @@ public class MessagingControllerPushReceiver implements PushReceiver
             {
                 latch.countDown();
             }
-        });
+        }, folder);
 
         if (K9.DEBUG)
             Log.v(K9.LOG_TAG, "syncFolder(" + folder.getName() + ") about to await latch release");

src/com/fsck/k9/activity/FolderList.java

@@ -202,7 +202,7 @@ public class FolderList extends K9ListActivity
                 wakeLock.release();
             }
         };
-        MessagingController.getInstance(getApplication()).synchronizeMailbox(mAccount, folder.name, listener);
+        MessagingController.getInstance(getApplication()).synchronizeMailbox(mAccount, folder.name, listener, null);
         sendMail(mAccount);
     }
 

src/com/fsck/k9/activity/MessageList.java

@@ -1097,7 +1097,7 @@ public class MessageList
 
     private void checkMail(Account account, String folderName)
     {
-        mController.synchronizeMailbox(account, folderName, mAdapter.mListener);
+        mController.synchronizeMailbox(account, folderName, mAdapter.mListener, null);
         sendMail(account);
     }
 

src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java

@@ -109,7 +109,7 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
                         store.checkSettings();
 
                         MessagingController.getInstance(getApplication()).listFolders(mAccount, true, null);
-                        MessagingController.getInstance(getApplication()).synchronizeMailbox(mAccount, K9.INBOX , null);
+                        MessagingController.getInstance(getApplication()).synchronizeMailbox(mAccount, K9.INBOX , null, null);
 
                     }
                     if (mDestroyed)

src/com/fsck/k9/mail/PushReceiver.java

@@ -9,6 +9,7 @@ public interface PushReceiver
     public void syncFolder(Folder folder);
     public void messagesArrived(Folder folder, List<Message> mess);
     public void messagesFlagsChanged(Folder folder, List<Message> mess);
+    public void messagesRemoved(Folder folder, List<Message> mess);
     public String getPushState(String folderName);
     public void pushError(String errorMessage, Exception e);
     public void setPushActive(String folderName, boolean enabled);

src/com/fsck/k9/mail/store/ImapStore.java

@@ -39,6 +39,7 @@ import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.security.Security;
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -448,6 +449,8 @@ public class ImapStore extends Store
         private OpenMode mMode;
         private boolean mExists;
         private ImapStore store = null;
+        Map<Integer, String> msgSeqUidMap = new ConcurrentHashMap<Integer, String>();
+
 
         public ImapFolder(ImapStore nStore, String name)
         {
@@ -522,6 +525,7 @@ public class ImapStore extends Store
             // 2 OK [READ-WRITE] Select completed.
             try
             {
+                msgSeqUidMap.clear();
                 String command = String.format((mode == OpenMode.READ_WRITE ? "SELECT" : "EXAMINE") + " \"%s\"",
                                                encodeFolderName(getPrefixedName()));
 
@@ -967,6 +971,19 @@ public class ImapStore extends Store
             return search(searcher, listener);
         }
         
+        protected Message[] getMessagesFromUids(final List<String> mesgUids, final boolean includeDeleted, final MessageRetrievalListener listener)
+        throws MessagingException
+        {
+            ImapSearcher searcher = new ImapSearcher()
+            {
+                public List<ImapResponse> search() throws IOException, MessagingException
+                {
+                    return executeSimpleCommand(String.format("UID SEARCH UID %s" + (includeDeleted ? "" : " NOT DELETED"), Utility.combine(mesgUids.toArray(), ',')));
+                }
+            };
+            return search(searcher, listener);
+        }
+
         private Message[] search(ImapSearcher searcher, MessageRetrievalListener listener) throws MessagingException
         {
 
@@ -1152,6 +1169,22 @@ public class ImapStore extends Store
                     {
                         ImapList fetchList = (ImapList)response.getKeyedValue("FETCH");
                         String uid = fetchList.getKeyedString("UID");
+                        int msgSeq = response.getNumber(0);
+                        if (uid != null)
+                        {
+                            try
+                            {
+                                msgSeqUidMap.put(msgSeq, uid);
+                                if (K9.DEBUG)
+                                {
+                                    Log.v(K9.LOG_TAG, "Stored uid '" + uid + "' for msgSeq " + msgSeq + " into map " + msgSeqUidMap.toString());
+                                }
+                            }
+                            catch (Exception e)
+                            {
+                                Log.e(K9.LOG_TAG, "Unable to store uid '" + uid + "' for msgSeq " + msgSeq);
+                            }
+                        }
 
                         Message message = messageMap.get(uid);
                         if (message == null)
@@ -1371,7 +1404,7 @@ public class ImapStore extends Store
                 {
                     mMessageCount--;
                     if (K9.DEBUG)
-                        Log.d(K9.LOG_TAG, "Got untagged EXPUNGE with value " + mMessageCount + " for " + getLogId());
+                        Log.d(K9.LOG_TAG, "Got untagged EXPUNGE with mMessageCount " + mMessageCount + " for " + getLogId());
                 }
 //            if (response.size() > 1) {
 //                Object bracketedObj = response.get(1);
@@ -2588,6 +2621,7 @@ public class ImapStore extends Store
         final AtomicBoolean doneSent = new AtomicBoolean(false);
         final AtomicInteger delayTime = new AtomicInteger(NORMAL_DELAY_TIME);
         final AtomicInteger idleFailureCount = new AtomicInteger(0);
+        final AtomicBoolean needsPoll = new AtomicBoolean(false);
         List<ImapResponse> storedUntaggedResponses = new ArrayList<ImapResponse>();
         
         public ImapFolderPusher(ImapStore store, String name, PushReceiver nReceiver)
@@ -2650,7 +2684,7 @@ public class ImapStore extends Store
                                 Log.e(K9.LOG_TAG, "Unable to get oldUidNext for " + getLogId(), e);
                             }
                             ImapConnection oldConnection = mConnection;
-                            List<ImapResponse> responses = internalOpen(OpenMode.READ_ONLY);
+                            internalOpen(OpenMode.READ_ONLY);
                             if (mConnection == null)
                             {
                                 receiver.pushError("Could not establish connection for IDLE", null);
@@ -2664,12 +2698,11 @@ public class ImapStore extends Store
                                 throw new MessagingException("IMAP server is not IDLE capable:" + mConnection.toString());
                             }
 
-                            if (responses != null)
+                            if (mAccount.isPushPollOnConnect() && (mConnection != oldConnection || needsPoll.getAndSet(false) == true))
-                            {
-                                handleUntaggedResponses(responses);
-                            }
-                            if (mAccount.isPushPollOnConnect() && mConnection != oldConnection)
                             {
+                                List<ImapResponse> untaggedResponses = new ArrayList<ImapResponse>(storedUntaggedResponses);
+                                storedUntaggedResponses.clear();
+                                processUntaggedResponses(untaggedResponses);
                                 receiver.syncFolder(ImapFolderPusher.this);
                             }
                             int startUid = oldUidNext;
@@ -2842,10 +2875,11 @@ public class ImapStore extends Store
                 skipSync = true;
             }
             List<Integer> flagSyncMsgSeqs = new ArrayList<Integer>();
+            List<String> removeMsgUids = new LinkedList<String>();
 
             for (ImapResponse response : responses)
             {
-                oldMessageCount += processUntaggedResponse(oldMessageCount, response, flagSyncMsgSeqs);
+                oldMessageCount += processUntaggedResponse(oldMessageCount, response, flagSyncMsgSeqs, removeMsgUids);
             }
             if (skipSync == false)
             {
@@ -2865,6 +2899,10 @@ public class ImapStore extends Store
             {
                 syncMessages(flagSyncMsgSeqs);
             }
+            if (removeMsgUids.size() > 0)
+            {
+                removeMessages(removeMsgUids);
+            }
         }
 
         private void syncMessages(int end, boolean newArrivals) throws MessagingException
@@ -2939,7 +2977,45 @@ public class ImapStore extends Store
             }
         }
         
-        protected int processUntaggedResponse(int oldMessageCount, ImapResponse response, List<Integer> flagSyncMsgSeqs)
+        private void removeMessages(List<String> removeUids)
+        {
+            List<Message> messages = new ArrayList<Message>(removeUids.size());
+            
+            try
+            {
+                Message[] existingMessages = getMessagesFromUids(removeUids, true, null);
+                for (Message existingMessage : existingMessages)
+                {
+                    needsPoll.set(true);
+                    msgSeqUidMap.clear();
+                    String existingUid = existingMessage.getUid();
+                    Log.w(K9.LOG_TAG, "Message with UID " + existingUid + " still exists on server, not expunging");
+                    removeUids.remove(existingUid);
+                }
+                for (String uid : removeUids)
+                {
+                    ImapMessage message = new ImapMessage(uid, this);
+                    try
+                    {
+                        message.setFlagInternal(Flag.DELETED, true);
+                    }
+                    catch (MessagingException me)
+                    {
+                        Log.e(K9.LOG_TAG, "Unable to set DELETED flag on message " + message.getUid());
+                    }
+                    messages.add(message);
+                }
+                receiver.messagesRemoved(this, messages);
+            }
+            catch (Exception e)
+            {
+                Log.e(K9.LOG_TAG, "Cannot remove EXPUNGEd messages", e);
+                return;
+            }
+          
+        }
+
+        protected int processUntaggedResponse(int oldMessageCount, ImapResponse response, List<Integer> flagSyncMsgSeqs, List<String> removeMsgUids)
         {
             super.handleUntaggedResponse(response);
             int messageCountDelta = 0;
@@ -2950,7 +3026,9 @@ public class ImapStore extends Store
                     Object responseType = response.get(1);
                     if (ImapResponseParser.equalsIgnoreCase(responseType, "FETCH"))
                     {
+                        Log.i(K9.LOG_TAG, "Got FETCH " + response);
                         int msgSeq = response.getNumber(0);
+                        
                         if (K9.DEBUG)
                             Log.d(K9.LOG_TAG, "Got untagged FETCH for msgseq " + msgSeq + " for " + getLogId());
 
@@ -2984,6 +3062,39 @@ public class ImapStore extends Store
                             }
                         }
                         flagSyncMsgSeqs.addAll(newSeqs);
+                        
+                        
+                        List<Integer> msgSeqs = new ArrayList<Integer>(msgSeqUidMap.keySet());
+                        Collections.sort(msgSeqs);  // Have to do comparisons in order because of msgSeq reductions
+
+                        for (Integer msgSeqNumI : msgSeqs)
+                        {
+                            if (K9.DEBUG)
+                            {
+                                Log.v(K9.LOG_TAG, "Comparing EXPUNGEd msgSeq " + msgSeq + " to " + msgSeqNumI);
+                            }
+                            int msgSeqNum = msgSeqNumI;
+                            if (msgSeqNum == msgSeq)
+                            {
+                                String uid = msgSeqUidMap.get(msgSeqNum);
+                                if (K9.DEBUG)
+                                {
+                                    Log.d(K9.LOG_TAG, "Scheduling removal of UID " + uid + " because msgSeq " + msgSeqNum + " was expunged");
+                                }
+                                removeMsgUids.add(uid);
+                                msgSeqUidMap.remove(msgSeqNum);
+                            }
+                            else if (msgSeqNum > msgSeq)
+                            {
+                                String uid = msgSeqUidMap.get(msgSeqNum);
+                                if (K9.DEBUG)
+                                {
+                                    Log.d(K9.LOG_TAG, "Reducing msgSeq for UID " + uid + " from " + msgSeqNum + " to " + (msgSeqNum - 1));
+                                }
+                                msgSeqUidMap.remove(msgSeqNum);
+                                msgSeqUidMap.put(msgSeqNum-1, uid);
+                            }
+                        }
                     }
                 }
                 catch (Exception e)