From a6212c88e0fb5906cb4065c775c1f4c0b83b7ca4 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Thu, 30 Jun 2011 12:01:29 -0700 Subject: [PATCH] Fix bug 4978035 Precondition failure: Not combined mailbox Also added test. (For some reason the message count doesn't get set properly on tests... I'll investigate it when I get around to it...) Change-Id: I83f3b6f2079da06b2d4973419d2296e6492de1d3 --- src/com/android/email/FolderProperties.java | 16 +- .../activity/AccountSelectorAdapter.java | 14 +- .../activity/MailboxFragmentAdapter.java | 2 +- ...ountSelectorAdapterAccountsLoaderTest.java | 75 ------- .../activity/AccountSelectorAdapterTest.java | 185 ++++++++++++++++++ 5 files changed, 206 insertions(+), 86 deletions(-) delete mode 100644 tests/src/com/android/email/activity/AccountSelectorAdapterAccountsLoaderTest.java create mode 100644 tests/src/com/android/email/activity/AccountSelectorAdapterTest.java diff --git a/src/com/android/email/FolderProperties.java b/src/com/android/email/FolderProperties.java index c5823c79b..e43e3ee47 100644 --- a/src/com/android/email/FolderProperties.java +++ b/src/com/android/email/FolderProperties.java @@ -176,20 +176,26 @@ public class FolderProperties { ); } - public int getMessageCountForCombinedMailbox(long mailboxId) { + /** + * @return message count to show for the UI for a combined inbox. + * + * Note this method doesn't use mContext so we can inject a mock context for provider + * access. So it's static. + */ + public static int getMessageCountForCombinedMailbox(Context context, long mailboxId) { Preconditions.checkState(mailboxId < -1L); if ((mailboxId == Mailbox.QUERY_ALL_INBOXES) || (mailboxId == Mailbox.QUERY_ALL_UNREAD)) { - return Mailbox.getUnreadCountByMailboxType(mContext, Mailbox.TYPE_INBOX); + return Mailbox.getUnreadCountByMailboxType(context, Mailbox.TYPE_INBOX); } else if (mailboxId == Mailbox.QUERY_ALL_FAVORITES) { - return Message.getFavoriteMessageCount(mContext); + return Message.getFavoriteMessageCount(context); } else if (mailboxId == Mailbox.QUERY_ALL_DRAFTS) { - return Mailbox.getMessageCountByMailboxType(mContext, Mailbox.TYPE_DRAFTS); + return Mailbox.getMessageCountByMailboxType(context, Mailbox.TYPE_DRAFTS); } else if (mailboxId == Mailbox.QUERY_ALL_OUTBOX) { - return Mailbox.getMessageCountByMailboxType(mContext, Mailbox.TYPE_OUTBOX); + return Mailbox.getMessageCountByMailboxType(context, Mailbox.TYPE_OUTBOX); } throw new IllegalStateException("Invalid mailbox ID"); } diff --git a/src/com/android/email/activity/AccountSelectorAdapter.java b/src/com/android/email/activity/AccountSelectorAdapter.java index 3f6124e02..8d7f1faa1 100644 --- a/src/com/android/email/activity/AccountSelectorAdapter.java +++ b/src/com/android/email/activity/AccountSelectorAdapter.java @@ -480,7 +480,8 @@ public class AccountSelectorAdapter extends CursorAdapter { private String mMailboxDisplayName; private int mMailboxMessageCount; - private CursorWithExtras(String[] columnNames, Cursor innerCursor) { + @VisibleForTesting + CursorWithExtras(String[] columnNames, Cursor innerCursor) { super(columnNames, innerCursor); } @@ -495,7 +496,8 @@ public class AccountSelectorAdapter extends CursorAdapter { /** * Set the current account/mailbox info. */ - private void setAccountMailboxInfo(Context context, long accountId, long mailboxId) { + @VisibleForTesting + void setAccountMailboxInfo(Context context, long accountId, long mailboxId) { mAccountId = accountId; mMailboxId = mailboxId; @@ -504,7 +506,9 @@ public class AccountSelectorAdapter extends CursorAdapter { // We need to treat ACCOUNT_ID_COMBINED_VIEW specially... mAccountExists = true; mAccountDisplayName = getCombinedViewDisplayName(context); - setCombinedMailboxInfo(context, mailboxId); + if (mailboxId != Mailbox.NO_MAILBOX) { + setCombinedMailboxInfo(context, mailboxId); + } return; } @@ -551,8 +555,8 @@ public class AccountSelectorAdapter extends CursorAdapter { mMailboxDisplayName = FolderProperties.getInstance(context) .getCombinedMailboxName(mMailboxId); - mMailboxMessageCount = FolderProperties.getInstance(context) - .getMessageCountForCombinedMailbox(mailboxId); + mMailboxMessageCount = FolderProperties.getMessageCountForCombinedMailbox( + context, mailboxId); } /** diff --git a/src/com/android/email/activity/MailboxFragmentAdapter.java b/src/com/android/email/activity/MailboxFragmentAdapter.java index c0353644e..3c4766ae1 100644 --- a/src/com/android/email/activity/MailboxFragmentAdapter.java +++ b/src/com/android/email/activity/MailboxFragmentAdapter.java @@ -472,7 +472,7 @@ class MailboxFragmentAdapter extends CursorAdapter { if (id >= 0) { throw new IllegalArgumentException(); // Must be QUERY_ALL_*, which are all negative } - int count = FolderProperties.getInstance(context).getMessageCountForCombinedMailbox(id); + int count = FolderProperties.getMessageCountForCombinedMailbox(context, id); if (showAlways || (count > 0)) { addMailboxRow( cursor, id, "", mailboxType, count, count, ROW_TYPE_MAILBOX, Mailbox.FLAG_NONE, diff --git a/tests/src/com/android/email/activity/AccountSelectorAdapterAccountsLoaderTest.java b/tests/src/com/android/email/activity/AccountSelectorAdapterAccountsLoaderTest.java deleted file mode 100644 index 16e258ad7..000000000 --- a/tests/src/com/android/email/activity/AccountSelectorAdapterAccountsLoaderTest.java +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright (C) 2010 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.email.activity; - -import com.android.email.DBTestHelper; -import com.android.email.provider.ProviderTestUtils; -import com.android.emailcommon.provider.Account; - -import android.content.Context; -import android.content.Loader; -import android.database.Cursor; -import android.test.LoaderTestCase; - -/** - * Tests for {@link AccountSelectorAdapter.AccountsLoader}. - * - * TODO add more tests. - */ -public class AccountSelectorAdapterAccountsLoaderTest extends LoaderTestCase { - private Context mProviderContext; - - @Override - protected void setUp() throws Exception { - super.setUp(); - mProviderContext = DBTestHelper.ProviderContextSetupHelper.getProviderContext(mContext); - } - - /** - * - Confirm that AccountsLoader adds the combined view row, iif there is more than 1 account. - * - Confirm that AccountsLoader doesn't add recent mailboxes. - * - * two-pane version. - * - * TODO add one-pane version - */ - public void testCombinedViewRow_twoPane() { - final Account a1 = ProviderTestUtils.setupAccount("a1", true, mProviderContext); - { - // Only 1 account -- no combined view row. - Loader l = new AccountSelectorAdapter.AccountsLoader(mProviderContext, 0L, - 0L, true); - AccountSelectorAdapter.CursorWithExtras result = - (AccountSelectorAdapter.CursorWithExtras) getLoaderResultSynchronously(l); - assertEquals(1, result.getAccountCount()); - assertEquals(2, result.getCount()); // +1 as the cursor has the header row - assertEquals(0, result.getRecentMailboxCount()); // No recent on two-pane. - } - - final Account a2 = ProviderTestUtils.setupAccount("a2", true, mProviderContext); - { - // 2 accounts -- with combined view row, so returns 3 account rows. - Loader l = new AccountSelectorAdapter.AccountsLoader(mProviderContext, 0L, - 0L, true); - AccountSelectorAdapter.CursorWithExtras result = - (AccountSelectorAdapter.CursorWithExtras) getLoaderResultSynchronously(l); - assertEquals(3, result.getAccountCount()); - assertEquals(4, result.getCount()); // +1 as the cursor has the header row - assertEquals(0, result.getRecentMailboxCount()); // No recent on two-pane. - } - } -} diff --git a/tests/src/com/android/email/activity/AccountSelectorAdapterTest.java b/tests/src/com/android/email/activity/AccountSelectorAdapterTest.java new file mode 100644 index 000000000..43da41f27 --- /dev/null +++ b/tests/src/com/android/email/activity/AccountSelectorAdapterTest.java @@ -0,0 +1,185 @@ +/* + * Copyright (C) 2010 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.email.activity; + +import com.android.email.DBTestHelper; +import com.android.email.FolderProperties; +import com.android.email.R; +import com.android.email.provider.ProviderTestUtils; +import com.android.emailcommon.provider.Account; +import com.android.emailcommon.provider.Mailbox; + +import android.content.Context; +import android.content.Loader; +import android.database.Cursor; +import android.database.MatrixCursor; +import android.test.LoaderTestCase; + +/** + * Tests for {@link AccountSelectorAdapter.AccountsLoader}. + * + * TODO add more tests. + */ +public class AccountSelectorAdapterTest extends LoaderTestCase { + private Context mProviderContext; + + @Override + protected void setUp() throws Exception { + super.setUp(); + mProviderContext = DBTestHelper.ProviderContextSetupHelper.getProviderContext(mContext); + } + + /** + * - Confirm that AccountsLoader adds the combined view row, iif there is more than 1 account. + * - Confirm that AccountsLoader doesn't add recent mailboxes. + * + * two-pane version. + * + * TODO add one-pane version + */ + public void testCombinedViewRow_twoPane() { + final Account a1 = ProviderTestUtils.setupAccount("a1", true, mProviderContext); + { + // Only 1 account -- no combined view row. + Loader l = new AccountSelectorAdapter.AccountsLoader(mProviderContext, 0L, + 0L, true); + AccountSelectorAdapter.CursorWithExtras result = + (AccountSelectorAdapter.CursorWithExtras) getLoaderResultSynchronously(l); + assertEquals(1, result.getAccountCount()); + assertEquals(2, result.getCount()); // +1 as the cursor has the header row + assertEquals(0, result.getRecentMailboxCount()); // No recent on two-pane. + } + + final Account a2 = ProviderTestUtils.setupAccount("a2", true, mProviderContext); + { + // 2 accounts -- with combined view row, so returns 3 account rows. + Loader l = new AccountSelectorAdapter.AccountsLoader(mProviderContext, 0L, + 0L, true); + AccountSelectorAdapter.CursorWithExtras result = + (AccountSelectorAdapter.CursorWithExtras) getLoaderResultSynchronously(l); + assertEquals(3, result.getAccountCount()); + assertEquals(4, result.getCount()); // +1 as the cursor has the header row + assertEquals(0, result.getRecentMailboxCount()); // No recent on two-pane. + } + } + + private static AccountSelectorAdapter.CursorWithExtras createCursorWithExtras() { + final MatrixCursor m = new MatrixCursor(new String[] {"column"}); + return new AccountSelectorAdapter.CursorWithExtras(m.getColumnNames(), m); + } + + public void testCursorWithExtras_setAccountMailboxInfo() { + final Context context = mProviderContext; + final Account a1 = ProviderTestUtils.setupAccount("a1", true, context); + final Account a2 = ProviderTestUtils.setupAccount("a2", true, context); + final Mailbox m1 = ProviderTestUtils.setupMailbox("Inbox", a1.mId, true, context, + Mailbox.TYPE_INBOX); + final Mailbox m2 = ProviderTestUtils.setupMailbox("box2", a2.mId, true, context, + Mailbox.TYPE_MAIL); + addMessage(m1, true, false); + addMessage(m2, false, false); + addMessage(m2, false, false); + addMessage(m2, true, true); + + // Account 1 - no mailbox + AccountSelectorAdapter.CursorWithExtras c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, a1.mId, Mailbox.NO_MAILBOX); + + assertTrue(c.accountExists()); + assertEquals(a1.mId, c.getAccountId()); + assertEquals("a1", c.getAccountDisplayName()); + assertEquals(Mailbox.NO_MAILBOX, c.getMailboxId()); + assertNull(c.getMailboxDisplayName()); + assertEquals(0, c.getMailboxMessageCount()); + + // Account 1 - inbox + c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, a1.mId, m1.mId); + + assertTrue(c.accountExists()); + assertEquals(a1.mId, c.getAccountId()); + assertEquals("a1", c.getAccountDisplayName()); + assertEquals(m1.mId, c.getMailboxId()); + assertEquals("Inbox", c.getMailboxDisplayName()); + assertEquals(1, c.getMailboxMessageCount()); + + // Account 2 - regular mailbox + c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, a2.mId, m2.mId); + + assertTrue(c.accountExists()); + assertEquals(a2.mId, c.getAccountId()); + assertEquals("a2", c.getAccountDisplayName()); + assertEquals(m2.mId, c.getMailboxId()); + assertEquals("box2", c.getMailboxDisplayName()); + assertEquals(2, c.getMailboxMessageCount()); + + // combined - no mailbox + c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, Account.ACCOUNT_ID_COMBINED_VIEW, Mailbox.NO_MAILBOX); + + assertTrue(c.accountExists()); + assertEquals(Account.ACCOUNT_ID_COMBINED_VIEW, c.getAccountId()); + assertEquals(getContext().getString(R.string.mailbox_list_account_selector_combined_view), + c.getAccountDisplayName()); + assertEquals(Mailbox.NO_MAILBOX, c.getMailboxId()); + assertNull(c.getMailboxDisplayName()); + assertEquals(0, c.getMailboxMessageCount()); + + // combined - all inbox + c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, Account.ACCOUNT_ID_COMBINED_VIEW, + Mailbox.QUERY_ALL_INBOXES); + + assertTrue(c.accountExists()); + assertEquals(Account.ACCOUNT_ID_COMBINED_VIEW, c.getAccountId()); + assertEquals(getContext().getString(R.string.mailbox_list_account_selector_combined_view), + c.getAccountDisplayName()); + assertEquals(Mailbox.QUERY_ALL_INBOXES, c.getMailboxId()); + assertEquals(getContext().getString(R.string.account_folder_list_summary_inbox), + c.getMailboxDisplayName()); + // (message count = 1, because account 2 doesn't have inbox) + +// TODO For some reason getMailboxMessageCount returns 0 in tests. Investigate it. +// assertEquals(1, c.getMailboxMessageCount()); + + // Account 1 - all starred + // Special case; it happens when you open "starred" on a normal account's mailbox list + // on two-pane. + c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, a1.mId, Mailbox.QUERY_ALL_FAVORITES); + + assertTrue(c.accountExists()); + assertEquals(a1.mId, c.getAccountId()); + assertEquals("a1", c.getAccountDisplayName()); + assertEquals(Mailbox.QUERY_ALL_FAVORITES, c.getMailboxId()); + assertEquals(getContext().getString(R.string.account_folder_list_summary_starred), + c.getMailboxDisplayName()); +// assertEquals(2, c.getMailboxMessageCount()); + + // Invalid id + c = createCursorWithExtras(); + c.setAccountMailboxInfo(context, 123456, 1232456); // no such account / mailbox + + assertFalse(c.accountExists()); + } + + private void addMessage(Mailbox m, boolean starred, boolean read) { + ProviderTestUtils.setupMessage("a", m.mAccountKey, m.mId, false, true, mProviderContext, + starred, read); + } +}