From de196ca3da05ee1026cc7b5e26aaee7e219a63f1 Mon Sep 17 00:00:00 2001 From: Marc Blank Date: Fri, 14 Jan 2011 15:33:44 -0800 Subject: [PATCH] When syncing folders, read all data first, then process Bug: 3353035 Change-Id: I80212b225eae48d0351f47f0d601f77578d2fc96 --- src/com/android/exchange/EasSyncService.java | 8 ++ src/com/android/exchange/ExchangeService.java | 2 +- .../exchange/adapter/FolderSyncParser.java | 107 ++++++++++-------- 3 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/com/android/exchange/EasSyncService.java b/src/com/android/exchange/EasSyncService.java index 3b0c173a8..2a1560d24 100644 --- a/src/com/android/exchange/EasSyncService.java +++ b/src/com/android/exchange/EasSyncService.java @@ -1883,6 +1883,7 @@ public class EasSyncService extends AbstractSyncService { ArrayList readyMailboxes = new ArrayList(); ArrayList notReadyMailboxes = new ArrayList(); int pingWaitCount = 0; + long inboxId = -1; while ((System.currentTimeMillis() < endTime) && !mStop) { // Count of pushable mailboxes @@ -1898,6 +1899,10 @@ public class EasSyncService extends AbstractSyncService { AND_FREQUENCY_PING_PUSH_AND_NOT_ACCOUNT_MAILBOX, null, null); notReadyMailboxes.clear(); readyMailboxes.clear(); + // Look for an inbox, and remember its id + if (inboxId == -1) { + inboxId = Mailbox.findMailboxOfType(mContext, mAccount.mId, Mailbox.TYPE_INBOX); + } try { // Loop through our pushed boxes seeing what is available to push while (c.moveToNext()) { @@ -2082,6 +2087,9 @@ public class EasSyncService extends AbstractSyncService { // we're in one of the other possible states. userLog("pingLoop waiting for initial sync of ", uninitCount, " box(es)"); sleep(10*SECONDS, true); + } else if (inboxId == -1) { + // In this case, we're still syncing mailboxes, so sleep for only a short time + sleep(45*SECONDS, true); } else { // We've got nothing to do, so we'll check again in 20 minutes at which time // we'll update the folder list, check for policy changes and/or remote wipe, etc. diff --git a/src/com/android/exchange/ExchangeService.java b/src/com/android/exchange/ExchangeService.java index 3d1466b93..e9e2af7e4 100644 --- a/src/com/android/exchange/ExchangeService.java +++ b/src/com/android/exchange/ExchangeService.java @@ -2336,7 +2336,7 @@ public class ExchangeService extends Service implements Runnable { } // DO NOT CALL THIS IN A LOOP ON THE SERVICEMAP - static private void stopManualSync(long mailboxId) { + static public void stopManualSync(long mailboxId) { ExchangeService exchangeService = INSTANCE; if (exchangeService == null) return; synchronized (sSyncLock) { diff --git a/src/com/android/exchange/adapter/FolderSyncParser.java b/src/com/android/exchange/adapter/FolderSyncParser.java index 0d36a0e18..ae20fd958 100644 --- a/src/com/android/exchange/adapter/FolderSyncParser.java +++ b/src/com/android/exchange/adapter/FolderSyncParser.java @@ -20,11 +20,11 @@ package com.android.exchange.adapter; import com.android.email.Utility; import com.android.email.provider.AttachmentProvider; import com.android.email.provider.EmailContent; -import com.android.email.provider.EmailProvider; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.AccountColumns; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.MailboxColumns; +import com.android.email.provider.EmailProvider; import com.android.exchange.Eas; import com.android.exchange.ExchangeService; import com.android.exchange.MockParserStream; @@ -361,9 +361,9 @@ public class FolderSyncParser extends AbstractSyncParser { } } - private void commitMailboxes(ArrayList validMailboxes, + private boolean commitMailboxes(ArrayList validMailboxes, ArrayList userMailboxes, HashMap mailboxMap, - ArrayList ops) throws IOException { + ArrayList ops) { // Go through the generic user mailboxes; we'll call them valid if any parent is valid for (Mailbox m: userMailboxes) { @@ -388,44 +388,26 @@ public class FolderSyncParser extends AbstractSyncParser { // If it IS repeatable, there's no good result, since the folder list will be invalid try { mContentResolver.applyBatch(EmailProvider.EMAIL_AUTHORITY, mOperations); + return true; } catch (RemoteException e) { - throw new IOException("RemoteException committing folders."); + userLog("RemoteException in commitMailboxes"); + return false; } catch (OperationApplicationException e) { - throw new IOException("OperationApplicationException committing folders."); + userLog("OperationApplicationException in commitMailboxes"); + return false; } } - public void changesParser(ArrayList ops, boolean initialSync) - throws IOException { - // Mailboxes that we known contain email - ArrayList validMailboxes = new ArrayList(); - // Mailboxes that we're unsure about - ArrayList userMailboxes = new ArrayList(); - // Maps folder serverId to mailbox type - HashMap mailboxMap = new HashMap(); + public void changesParser(final ArrayList ops, + final boolean initialSync) throws IOException { + // Array of added mailboxes + final ArrayList addMailboxes = new ArrayList(); - int mailboxAddCount = 0; while (nextTag(Tags.FOLDER_CHANGES) != END) { if (tag == Tags.FOLDER_ADD) { Mailbox mailbox = addParser(); if (mailbox != null) { - // Save away the type of this folder - mailboxMap.put(mailbox.mServerId, mailbox); - // And add the mailbox to the proper list - if (type == USER_MAILBOX_TYPE) { - userMailboxes.add(mailbox); - } else { - validMailboxes.add(mailbox); - } - // On initial sync, we commit what we have every 20 mailboxes - if (initialSync && (++mailboxAddCount == MAILBOX_COMMIT_SIZE)) { - commitMailboxes(validMailboxes, userMailboxes, mailboxMap, ops); - // Clear our arrays to prepare for more - userMailboxes.clear(); - validMailboxes.clear(); - ops.clear(); - mailboxAddCount = 0; - } + addMailboxes.add(mailbox); } } else if (tag == Tags.FOLDER_DELETE) { deleteParser(ops); @@ -437,22 +419,53 @@ public class FolderSyncParser extends AbstractSyncParser { skipTag(); } - // The mock stream is used for junit tests, so that the parsing code can be tested - // separately from the provider code. - // TODO Change tests to not require this; remove references to the mock stream - if (mMock != null) { - mMock.setResult(null); - return; - } - - // Commit the sync key and mailboxes - ContentValues cv = new ContentValues(); - cv.put(AccountColumns.SYNC_KEY, mAccount.mSyncKey); - ops.add(ContentProviderOperation.newUpdate( - ContentUris.withAppendedId(Account.CONTENT_URI, mAccount.mId)) - .withValues(cv) - .build()); - commitMailboxes(validMailboxes, userMailboxes, mailboxMap, ops); + Utility.runAsync(new Runnable() { + @Override + public void run() { + // Synchronize on the parser to prevent this being run multiple times concurrently + // (an extremely unlikely event, but nonetheless possible) + synchronized (FolderSyncParser.this) { + // Mailboxes that we known contain email + ArrayList validMailboxes = new ArrayList(); + // Mailboxes that we're unsure about + ArrayList userMailboxes = new ArrayList(); + // Maps folder serverId to mailbox type + HashMap mailboxMap = new HashMap(); + int mailboxCommitCount = 0; + for (Mailbox mailbox : addMailboxes) { + // Save away the type of this folder + mailboxMap.put(mailbox.mServerId, mailbox); + // And add the mailbox to the proper list + if (type == USER_MAILBOX_TYPE) { + userMailboxes.add(mailbox); + } else { + validMailboxes.add(mailbox); + } + // On initial sync, we commit what we have every 20 mailboxes + if (initialSync && (++mailboxCommitCount == MAILBOX_COMMIT_SIZE)) { + if (!commitMailboxes(validMailboxes, userMailboxes, mailboxMap, ops)) { + mService.stop(); + return; + } + // Clear our arrays to prepare for more + userMailboxes.clear(); + validMailboxes.clear(); + ops.clear(); + mailboxCommitCount = 0; + } + } + // Commit the sync key and mailboxes + ContentValues cv = new ContentValues(); + cv.put(AccountColumns.SYNC_KEY, mAccount.mSyncKey); + ops.add(ContentProviderOperation + .newUpdate( + ContentUris.withAppendedId(Account.CONTENT_URI, mAccount.mId)) + .withValues(cv).build()); + if (!commitMailboxes(validMailboxes, userMailboxes, mailboxMap, ops)) { + mService.stop(); + } + } + }}); } /**