Update search folder status in a finally block

Also clean up a dangerous logging call and clean up some warnings

b/17283951

Change-Id: Ia295218e5ee1162dac68c5a8e7eff6a9dd74a918

src/com/android/email/service/ImapService.java

@@ -1457,8 +1457,8 @@ public class ImapService extends Service {
         final Mailbox mailbox = Mailbox.restoreMailboxWithId(context, searchParams.mMailboxId);
         final Mailbox destMailbox = Mailbox.restoreMailboxWithId(context, destMailboxId);
         if (account == null || mailbox == null || destMailbox == null) {
-            LogUtils.d(Logging.LOG_TAG, "Attempted search for " + searchParams
+            LogUtils.d(Logging.LOG_TAG, "Attempted search for %s "
-                    + " but account or mailbox information was missing");
+                    + "but account or mailbox information was missing", searchParams);
             return 0;
         }
 
@@ -1467,7 +1467,10 @@ public class ImapService extends Service {
         statusValues.put(Mailbox.UI_SYNC_STATUS, UIProvider.SyncStatus.LIVE_QUERY);
         destMailbox.update(context, statusValues);
 
-        final Store remoteStore = Store.getInstance(account, context);
+        Store remoteStore = null;
+        int numSearchResults = 0;
+        try {
+            remoteStore = Store.getInstance(account, context);
             final Folder remoteFolder = remoteStore.getFolder(mailbox.mServerId);
             remoteFolder.open(OpenMode.READ_WRITE);
 
@@ -1480,11 +1483,12 @@ public class ImapService extends Service {
                     sortableMessages = new SortableMessage[remoteCount];
                     int i = 0;
                     for (Message msg : remoteMessages) {
-                    sortableMessages[i++] = new SortableMessage(msg, Long.parseLong(msg.getUid()));
+                        sortableMessages[i++] = new SortableMessage(msg,
+                                Long.parseLong(msg.getUid()));
                     }
                     // Sort the uid's, most recent first
-                // Note: Not all servers will be nice and return results in the order of request;
+                    // Note: Not all servers will be nice and return results in the order of
-                // those that do will see messages arrive from newest to oldest
+                    // request; those that do will see messages arrive from newest to oldest
                     Arrays.sort(sortableMessages, new Comparator<SortableMessage>() {
                         @Override
                         public int compare(SortableMessage lhs, SortableMessage rhs) {
@@ -1500,19 +1504,15 @@ public class ImapService extends Service {
                 sortableMessages = sSearchResults.get(accountId);
             }
 
-        final int numSearchResults = (sortableMessages != null ? sortableMessages.length : 0);
+            numSearchResults = (sortableMessages != null ? sortableMessages.length : 0);
             final int numToLoad =
                     Math.min(numSearchResults - searchParams.mOffset, searchParams.mLimit);
             destMailbox.updateMessageCount(context, numSearchResults);
             if (numToLoad <= 0) {
-            // Tell UI that we're done loading messages
-            statusValues.put(Mailbox.SYNC_TIME, System.currentTimeMillis());
-            statusValues.put(Mailbox.UI_SYNC_STATUS, UIProvider.SyncStatus.NO_SYNC);
-            destMailbox.update(context, statusValues);
                 return 0;
             }
 
-        final ArrayList<Message> messageList = new ArrayList<Message>();
+            final ArrayList<Message> messageList = new ArrayList<>();
             for (int i = searchParams.mOffset; i < numToLoad + searchParams.mOffset; i++) {
                 messageList.add(sortableMessages[i].mMessage);
             }
@@ -1524,19 +1524,19 @@ public class ImapService extends Service {
 
             Message[] messageArray = messageList.toArray(new Message[messageList.size()]);
 
-        // TODO: We are purposely processing messages with a MessageRetrievalListener here, rather
+            // TODO: We are purposely processing messages with a MessageRetrievalListener here,
-        // than just walking the messageArray after the operation completes. This is so that we can
+            // rather than just walking the messageArray after the operation completes. This is so
-        // immediately update the database so the user can see something useful happening, even
+            // that we can immediately update the database so the user can see something useful
-        // if the message body has not yet been fetched.
+            // happening, even if the message body has not yet been fetched.
             // There are some issues with this approach:
-        // 1. It means that we have a single thread doing both network and database operations, and
+            // 1. It means that we have a single thread doing both network and database operations,
-        // either can block the other. The database updates could slow down the network reads,
+            // and either can block the other. The database updates could slow down the network
-        // keeping our network connection open longer than is really necessary.
+            // reads, keeping our network connection open longer than is really necessary.
             // 2. We still load all of this data into messageArray, even though it's not used.
             // It would be nicer if we had one thread doing the network operation, and a separate
-        // thread consuming that data and performing the appropriate database work, then discarding
+            // thread consuming that data and performing the appropriate database work, then
-        // the data as soon as it is no longer needed. This would reduce our memory footprint and
+            // discarding the data as soon as it is no longer needed. This would reduce our memory
-        // potentially allow our network operation to complete faster.
+            // footprint and potentially allow our network operation to complete faster.
             remoteFolder.fetch(messageArray, fp, new MessageRetrievalListener() {
                 @Override
                 public void messageRetrieved(Message message) {
@@ -1585,8 +1585,8 @@ public class ImapService extends Service {
             for (Message message : messageArray) {
                 // Build a list of parts we are interested in. Text parts will be downloaded
                 // right now, attachments will be left for later.
-            ArrayList<Part> viewables = new ArrayList<Part>();
+                ArrayList<Part> viewables = new ArrayList<>();
-            ArrayList<Part> attachments = new ArrayList<Part>();
+                ArrayList<Part> attachments = new ArrayList<>();
                 MimeUtility.collectParts(message, viewables, attachments);
                 // Download the viewables immediately
                 oneMessageArray[0] = message;
@@ -1600,12 +1600,15 @@ public class ImapService extends Service {
                         EmailContent.Message.FLAG_LOADED_COMPLETE);
             }
 
+        } finally {
+            if (remoteStore != null) {
+                remoteStore.closeConnections();
+            }
             // Tell UI that we're done loading messages
             statusValues.put(Mailbox.SYNC_TIME, System.currentTimeMillis());
             statusValues.put(Mailbox.UI_SYNC_STATUS, UIProvider.SyncStatus.NO_SYNC);
             destMailbox.update(context, statusValues);
-
+        }
-        remoteStore.closeConnections();
 
         return numSearchResults;
     }