From e357f5879187124c7af5c2ece5d7d3e4f60f07d2 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Fri, 27 Aug 2010 10:25:03 -0700 Subject: [PATCH] Fully implement "refresh" action bar button * Now it'll refresh mailbox list (left pane) as well. (With the minimal interval of 30 seconds) * Always refresh inbox. (also with the minimal interval of 10 seconds) * Also make sure the "auto-refresh" won't refresh non-refreshable mailboxes. (drafts, etc) Bug 2929889 Bug 2930018 Change-Id: I09452d40aad6008a721cfbc3f491617224d7048f --- src/com/android/email/RefreshManager.java | 26 ++- .../email/activity/MessageListFragment.java | 16 +- .../android/email/activity/MessageListXL.java | 132 ++++++++++-- .../email/data/MailboxAccountLoader.java | 49 +++-- .../android/email/provider/EmailContent.java | 36 ++++ .../MessageListXLRefreshTaskTest.java | 190 ++++++++++++++++++ .../data/MailboxAccountLoaderTestCase.java | 94 ++++++--- .../android/email/provider/ProviderTests.java | 35 ++++ 8 files changed, 508 insertions(+), 70 deletions(-) create mode 100644 tests/src/com/android/email/activity/MessageListXLRefreshTaskTest.java diff --git a/src/com/android/email/RefreshManager.java b/src/com/android/email/RefreshManager.java index ce4d0a1b4..5b4e8b64b 100644 --- a/src/com/android/email/RefreshManager.java +++ b/src/com/android/email/RefreshManager.java @@ -17,10 +17,8 @@ package com.android.email; import com.android.email.mail.MessagingException; -import com.android.email.provider.EmailContent; import android.content.Context; -import android.database.Cursor; import android.os.Handler; import android.util.Log; @@ -45,7 +43,7 @@ import java.util.HashMap; * *

Conceptually it can be a part of {@link Controller}, but extracted for easy testing. * - * (All public method musb be called on the UI thread. All callbacks will be called on the UI + * (All public methods must be called on the UI thread. All callbacks will be called on the UI * thread.) */ public class RefreshManager { @@ -159,7 +157,7 @@ public class RefreshManager { return sInstance; } - /* package */ RefreshManager(Context context, Controller controller, Clock clock, + protected RefreshManager(Context context, Controller controller, Clock clock, Handler handler) { mClock = clock; mContext = context.getApplicationContext(); @@ -169,6 +167,14 @@ public class RefreshManager { mController.addResultCallback(mControllerResult); } + /** + * MUST be called for mock instances. (The actual instance is a singleton, so no cleanup + * is necessary.) + */ + public void cleanUpForTest() { + mController.removeResultCallback(mControllerResult); + } + public void registerListener(Listener listener) { if (listener == null) { throw new InvalidParameterException(); @@ -261,6 +267,18 @@ public class RefreshManager { } } + public long getLastMailboxListRefreshTime(long accountId) { + return mMailboxListStatus.get(accountId).getLastRefreshTime(); + } + + public long getLastMessageListRefreshTime(long mailboxId) { + return mMessageListStatus.get(mailboxId).getLastRefreshTime(); + } + + public long getLastSendMessageTime(long accountId) { + return mOutboxStatus.get(accountId).getLastRefreshTime(); + } + public boolean isMailboxListRefreshing(long accountId) { return mMailboxListStatus.get(accountId).isRefreshing(); } diff --git a/src/com/android/email/activity/MessageListFragment.java b/src/com/android/email/activity/MessageListFragment.java index 569c105bc..c64dc7979 100644 --- a/src/com/android/email/activity/MessageListFragment.java +++ b/src/com/android/email/activity/MessageListFragment.java @@ -98,6 +98,7 @@ public class MessageListFragment extends ListFragment private Account mAccount; private Mailbox mMailbox; private boolean mIsEasAccount; + private boolean mIsRefreshable; // Controller access private Controller mController; @@ -306,7 +307,7 @@ public class MessageListFragment extends ListFragment } /** - * @return the mailbox id, which is the value set to {@link #openMailbox(long, long)}. + * @return the mailbox id, which is the value set to {@link #openMailbox}. * (Meaning it will never return -1, but may return special values, * eg {@link Mailbox#QUERY_ALL_INBOXES}). */ @@ -414,8 +415,13 @@ public class MessageListFragment extends ListFragment /** * Refresh the list. NOOP for special mailboxes (e.g. combined inbox). + * + * Note: Manual refresh is enabled even for push accounts. */ public void onRefresh() { + if (!mIsRefreshable) { + return; + } long accountId = getAccountId(); if (accountId != -1) { mRefreshManager.refreshMessageList(accountId, mMailboxId); @@ -646,13 +652,14 @@ public class MessageListFragment extends ListFragment /** * Implements a timed refresh of "stale" mailboxes. This should only happen when * multiple conditions are true, including: + * Only refreshable mailboxes. * Only when the user explicitly opens the mailbox (not onResume, for example) - * Only for real, non-push mailboxes + * Only for real, non-push mailboxes (c.f. manual refresh is still enabled for push accounts) * Only when the mailbox is "stale" (currently set to 5 minutes since last refresh) */ private void autoRefreshStaleMailbox() { if (!mDoAutoRefresh // Not explicitly open - || (mMailbox == null) // Magic inbox + || mIsRefreshable // Not refreshable (special box such as drafts, or magic boxes) || (mAccount.mSyncInterval == Account.CHECK_INTERVAL_PUSH) // Not push ) { return; @@ -788,7 +795,7 @@ public class MessageListFragment extends ListFragment Log.d(Email.LOG_TAG, "MessageListFragment onLoadFinished(mailbox) mailboxId=" + mMailboxId); } - if (!isMagicMailbox() && !result.isFound()) { + if (!result.mIsFound) { mCallback.onMailboxNotFound(); return; } @@ -797,6 +804,7 @@ public class MessageListFragment extends ListFragment mAccount = result.mAccount; mMailbox = result.mMailbox; mIsEasAccount = result.mIsEasAccount; + mIsRefreshable = result.mIsRefreshable; getLoaderManager().initLoader(LOADER_ID_MESSAGES_LOADER, null, new MessagesLoaderCallback()); } diff --git a/src/com/android/email/activity/MessageListXL.java b/src/com/android/email/activity/MessageListXL.java index 39f872fab..18167611c 100644 --- a/src/com/android/email/activity/MessageListXL.java +++ b/src/com/android/email/activity/MessageListXL.java @@ -16,12 +16,14 @@ package com.android.email.activity; +import com.android.email.Clock; import com.android.email.Email; import com.android.email.R; import com.android.email.RefreshManager; import com.android.email.Utility; import com.android.email.activity.setup.AccountSettingsXL; import com.android.email.activity.setup.AccountSetupBasics; +import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.service.MailService; @@ -33,6 +35,7 @@ import android.content.Context; import android.content.Intent; import android.content.Loader; import android.database.Cursor; +import android.os.AsyncTask; import android.os.Bundle; import android.util.Log; import android.view.KeyEvent; @@ -58,6 +61,8 @@ public class MessageListXL extends Activity implements View.OnClickListener, private static final String EXTRA_ACCOUNT_ID = "ACCOUNT_ID"; private static final String EXTRA_MAILBOX_ID = "MAILBOX_ID"; private static final int LOADER_ID_ACCOUNT_LIST = 0; + /* package */ static final int MAILBOX_REFRESH_MIN_INTERVAL = 30 * 1000; // in milliseconds + /* package */ static final int INBOX_AUTO_REFRESH_MIN_INTERVAL = 10 * 1000; // in milliseconds private Context mContext; private RefreshManager mRefreshManager; @@ -80,6 +85,8 @@ public class MessageListXL extends Activity implements View.OnClickListener, private final MessageOrderManagerCallback mMessageOrderManagerCallback = new MessageOrderManagerCallback(); + private RefreshTask mRefreshTask; + /** * Launch and open account's inbox. * @@ -210,6 +217,7 @@ public class MessageListXL extends Activity implements View.OnClickListener, @Override protected void onDestroy() { if (Email.DEBUG_LIFECYCLE && Email.DEBUG) Log.d(Email.LOG_TAG, "MessageListXL onDestroy"); + Utility.cancelTaskInterrupt(mRefreshTask); mRefreshManager.unregisterListener(mMailRefreshManagerListener); super.onDestroy(); } @@ -543,7 +551,6 @@ public class MessageListXL extends Activity implements View.OnClickListener, private boolean isProgressActive() { final long mailboxId = mFragmentManager.getMailboxId(); return (mailboxId >= 0) && mRefreshManager.isMessageListRefreshing(mailboxId); - } @Override @@ -604,29 +611,114 @@ public class MessageListXL extends Activity implements View.OnClickListener, } private void onRefresh() { - // Temporary implementation - if (mFragmentManager.isMailboxSelected()) { - long mailboxId = mFragmentManager.getMailboxId(); - // TODO This class here shouldn't really know what can be refreshable. - // (The test below is only to prevent a crash... It's not enough. e.g. no refresh - // for outboxes.) - if (mailboxId >= 0) { - mRefreshManager.refreshMessageList(mFragmentManager.getAccountId(), mailboxId); + // Cancel previously running instance if any. + Utility.cancelTaskInterrupt(mRefreshTask); + mRefreshTask = new RefreshTask(this, mFragmentManager.getAccountId(), + mFragmentManager.getMailboxId()); + mRefreshTask.execute(); + } + + /** + * Class to handle refresh. + * + * When the user press "refresh", + *

+ */ + /* package */ static class RefreshTask extends AsyncTask { + private final Clock mClock; + private final Context mContext; + private final long mAccountId; + private final long mMailboxId; + private final RefreshManager mRefreshManager; + /* package */ long mInboxId; + + public RefreshTask(Context context, long accountId, long mailboxId) { + this(context, accountId, mailboxId, Clock.INSTANCE, + RefreshManager.getInstance(context)); + } + + /* package */ RefreshTask(Context context, long accountId, long mailboxId, Clock clock, + RefreshManager refreshManager) { + mClock = clock; + mContext = context; + mRefreshManager = refreshManager; + mAccountId = accountId; + mMailboxId = mailboxId; + } + + /** + * Do DB access on a worker thread. + */ + @Override + protected Boolean doInBackground(Void... params) { + mInboxId = Account.getInboxId(mContext, mAccountId); + return Mailbox.isRefreshable(mContext, mMailboxId); + } + + /** + * Do the actual refresh. + */ + @Override + protected void onPostExecute(Boolean isCurrentMailboxRefreshable) { + if (isCancelled() || isCurrentMailboxRefreshable == null) { + return; + } + if (isCurrentMailboxRefreshable) { + mRefreshManager.refreshMessageList(mAccountId, mMailboxId); + } + // Refresh mailbox list + if (mAccountId != -1) { + if (shouldRefreshMailboxList()) { + mRefreshManager.refreshMailboxList(mAccountId); + } + } + // Refresh inbox + if (shouldAutoRefreshInbox()) { + mRefreshManager.refreshMessageList(mAccountId, mInboxId); } } - // TODO implement this - // - Refresh mailbox list. But don't do that always; implement a min interval. - // - // - Refresh the selected mailbox, if it's supported. - // (regardless if the right-pane is MessageList or MessageView) - // - If not suppoted (e.g. outbox, draft, or push mailboxes), refresh the inbox of the - // current account. + /** + * @return true if the mailbox list of the current account hasn't been refreshed + * in the last {@link #MAILBOX_REFRESH_MIN_INTERVAL}. + */ + /* package */ boolean shouldRefreshMailboxList() { + if (mRefreshManager.isMailboxListRefreshing(mAccountId)) { + return false; + } + final long nextRefreshTime = mRefreshManager.getLastMailboxListRefreshTime(mAccountId) + + MAILBOX_REFRESH_MIN_INTERVAL; + if (nextRefreshTime > mClock.getTime()) { + return false; + } + return true; + } - // To do that, we need a way to tell the type of the currently selected mailbox. - // We can do this with MessageListFragment, but it's gone it if a message is being viewed. - // Maybe we should always have a MessageListFragment instance? - // That way it'll be easier to restore the scroll position. + /** + * @return true if the inbox of the current account hasn't been refreshed + * in the last {@link #INBOX_AUTO_REFRESH_MIN_INTERVAL}. + */ + /* package */ boolean shouldAutoRefreshInbox() { + if (mInboxId == mMailboxId) { + return false; // Current ID == inbox. No need to auto-refresh. + } + if (mRefreshManager.isMessageListRefreshing(mInboxId)) { + return false; + } + final long nextRefreshTime = mRefreshManager.getLastMessageListRefreshTime(mInboxId) + + INBOX_AUTO_REFRESH_MIN_INTERVAL; + if (nextRefreshTime > mClock.getTime()) { + return false; + } + return true; + } } /** diff --git a/src/com/android/email/data/MailboxAccountLoader.java b/src/com/android/email/data/MailboxAccountLoader.java index a527e08ba..645b8017a 100644 --- a/src/com/android/email/data/MailboxAccountLoader.java +++ b/src/com/android/email/data/MailboxAccountLoader.java @@ -22,18 +22,27 @@ import com.android.email.provider.EmailContent.Mailbox; import android.content.AsyncTaskLoader; import android.content.Context; +import java.security.InvalidParameterException; + /** * Loader to load {@link Mailbox} and {@link Account}. */ public class MailboxAccountLoader extends AsyncTaskLoader { public static class Result { - public Account mAccount; - public Mailbox mMailbox; - public boolean mIsEasAccount; + public final boolean mIsFound; + public final Account mAccount; + public final Mailbox mMailbox; + public final boolean mIsEasAccount; + public final boolean mIsRefreshable; - public boolean isFound() { - return (mAccount != null) && (mMailbox != null); + private Result(boolean found, Account account, Mailbox mailbox, boolean isEasAccount, + boolean isRefreshable) { + mIsFound = found; + mAccount = account; + mMailbox = mailbox; + mIsEasAccount = isEasAccount; + mIsRefreshable = isRefreshable; } } @@ -42,28 +51,38 @@ public class MailboxAccountLoader extends AsyncTaskLoader { ProviderTestUtils.assertMailboxEqual("x", b1, Mailbox.getMailboxForMessageId(c, m1.mId)); ProviderTestUtils.assertMailboxEqual("x", b2, Mailbox.getMailboxForMessageId(c, m2.mId)); } + + public void testGetAccountGetInboxIdTest() { + final Context c = mMockContext; + + // Prepare some data with red-herrings. + Account a1 = ProviderTestUtils.setupAccount("acct1", true, c); + Account a2 = ProviderTestUtils.setupAccount("acct2", true, c); + Mailbox b1i = ProviderTestUtils.setupMailbox("b1i", a1.mId, true, c, Mailbox.TYPE_INBOX); + Mailbox b2a = ProviderTestUtils.setupMailbox("b2a", a2.mId, true, c, Mailbox.TYPE_MAIL); + Mailbox b2i = ProviderTestUtils.setupMailbox("b2b", a2.mId, true, c, Mailbox.TYPE_INBOX); + + assertEquals(b2i.mId, Account.getInboxId(c, a2.mId)); + } + + public void testMailboxIsRefreshable() { + final Context c = mMockContext; + + Account a = ProviderTestUtils.setupAccount("acct1", true, c); + Mailbox bi = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_INBOX); + Mailbox bm = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_MAIL); + Mailbox bd = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_DRAFTS); + Mailbox bo = ProviderTestUtils.setupMailbox("b1", a.mId, true, c, Mailbox.TYPE_OUTBOX); + + assertTrue(Mailbox.isRefreshable(c, bi.mId)); + assertTrue(Mailbox.isRefreshable(c, bm.mId)); + assertFalse(Mailbox.isRefreshable(c, bd.mId)); + assertFalse(Mailbox.isRefreshable(c, bo.mId)); + + // No such mailbox + assertFalse(Mailbox.isRefreshable(c, -1)); + + // Magic mailboxes can't be refreshed. + assertFalse(Mailbox.isRefreshable(c, Mailbox.QUERY_ALL_DRAFTS)); + assertFalse(Mailbox.isRefreshable(c, Mailbox.QUERY_ALL_INBOXES)); + } }