From 3a1e874a9cedf1a579c44d9342937777c8f21d18 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Mon, 6 Jun 2011 14:58:39 -0700 Subject: [PATCH] Fix filtered query we need to return 5, post filtered, results. previously, we were returning 5, pre filtered, results. Also add a test to catch this condition. Change-Id: Id25f4bf79081c42a2012e0e51b36142120c83b20 --- .../email/activity/RecentMailboxManager.java | 12 ++- .../activity/RecentMailboxManagerTest.java | 95 ++++++++++++++----- 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src/com/android/email/activity/RecentMailboxManager.java b/src/com/android/email/activity/RecentMailboxManager.java index c3dd43736..cb97286b9 100644 --- a/src/com/android/email/activity/RecentMailboxManager.java +++ b/src/com/android/email/activity/RecentMailboxManager.java @@ -36,6 +36,8 @@ import java.util.ArrayList; public class RecentMailboxManager { @VisibleForTesting static Clock sClock = Clock.INSTANCE; + @VisibleForTesting + static RecentMailboxManager sInstance; /** The maximum number of results to retrieve */ private static final int LIMIT_RESULTS = 5; @@ -50,9 +52,15 @@ public class RecentMailboxManager { + " LIMIT ? )"; /** Similar query to {@link #RECENT_SELECTION}, except, exclude all but user mailboxes */ private static final String RECENT_SELECTION_WITH_EXCLUSIONS = - RECENT_SELECTION + " AND " + MailboxColumns.TYPE + "=" + Mailbox.TYPE_MAIL; + MailboxColumns.ID + " IN " + + "( SELECT " + MailboxColumns.ID + + " FROM " + Mailbox.TABLE_NAME + + " WHERE ( " + MailboxColumns.ACCOUNT_KEY + "=? " + + " AND " + MailboxColumns.TYPE + "=" + Mailbox.TYPE_MAIL + + " AND " + MailboxColumns.LAST_TOUCHED_TIME + ">0 )" + + " ORDER BY " + MailboxColumns.LAST_TOUCHED_TIME + " DESC" + + " LIMIT ? )"; private final Context mContext; - private static RecentMailboxManager sInstance; public static synchronized RecentMailboxManager getInstance(Context context) { if (sInstance == null) { diff --git a/tests/src/com/android/email/activity/RecentMailboxManagerTest.java b/tests/src/com/android/email/activity/RecentMailboxManagerTest.java index 9788c3edd..e8a23124b 100644 --- a/tests/src/com/android/email/activity/RecentMailboxManagerTest.java +++ b/tests/src/com/android/email/activity/RecentMailboxManagerTest.java @@ -61,11 +61,19 @@ public class RecentMailboxManagerTest extends ProviderTestCase2 { ProviderTestUtils.setupMailbox("abbott", 1L, true, mMockContext, Mailbox.TYPE_MAIL), ProviderTestUtils.setupMailbox("costello", 1L, true, mMockContext, Mailbox.TYPE_MAIL), ProviderTestUtils.setupMailbox("bud_lou", 1L, true, mMockContext, Mailbox.TYPE_MAIL), + ProviderTestUtils.setupMailbox("laurel", 1L, true, mMockContext, Mailbox.TYPE_MAIL), + ProviderTestUtils.setupMailbox("hardy", 1L, true, mMockContext, Mailbox.TYPE_MAIL), }; // Invalidate all caches, since we reset the database for each test ContentCache.invalidateAllCachesForTest(); } + @Override + protected void tearDown() throws Exception { + RecentMailboxManager.sInstance = null; + super.tearDown(); + } + public void testTouch() throws Exception { // Ensure all accounts can be touched for (Mailbox mailbox : mMailboxArray) { @@ -92,7 +100,8 @@ public class RecentMailboxManagerTest extends ProviderTestCase2 { } } - public void testGetMostRecent() throws Exception { + /** Test default list */ + public void testGetMostRecent01() throws Exception { ArrayList testList; // test default list @@ -100,15 +109,16 @@ public class RecentMailboxManagerTest extends ProviderTestCase2 { assertEquals(0, testList.size()); testList = mManager.getMostRecent(1L, true); assertEquals(0, testList.size()); + } + /** Test recent list not full */ + public void testGetMostRecent02() throws Exception { + ArrayList testList; // touch some mailboxes - mManager.touch(mMailboxArray[0].mId); // inbox - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[3].mId); // sent - mMockClock.advance(1000L); + mMockClock.advance(1000L); mManager.touch(mMailboxArray[0].mId); // inbox + mMockClock.advance(1000L); mManager.touch(mMailboxArray[3].mId); // sent // need to wait for the last one to ensure getMostRecent() has something to work on - mManager.touch(mMailboxArray[7].mId).get(); // user mailbox #2 - mMockClock.advance(1000L); + mMockClock.advance(1000L); mManager.touch(mMailboxArray[7].mId).get(); // user mailbox #2 // test recent list not full testList = mManager.getMostRecent(1L, false); @@ -119,16 +129,18 @@ public class RecentMailboxManagerTest extends ProviderTestCase2 { testList = mManager.getMostRecent(1L, true); assertEquals(1, testList.size()); assertEquals(mMailboxArray[7].mId, (long) testList.get(0)); + } + + /** Test full recent list */ + public void testGetMostRecent03() throws Exception { + ArrayList testList; // touch some more mailboxes - mManager.touch(mMailboxArray[4].mId); // trash - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[2].mId); // outbox - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[8].mId); // user mailbox #3 - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[7].mId).get(); // user mailbox #2 - mMockClock.advance(1000L); + mMockClock.advance(1000L); mManager.touch(mMailboxArray[3].mId); // sent + mMockClock.advance(1000L); mManager.touch(mMailboxArray[4].mId); // trash + mMockClock.advance(1000L); mManager.touch(mMailboxArray[2].mId); // outbox + mMockClock.advance(1000L); mManager.touch(mMailboxArray[8].mId); // user mailbox #3 + mMockClock.advance(1000L); mManager.touch(mMailboxArray[7].mId).get(); // user mailbox #2 // test full recent list testList = mManager.getMostRecent(1L, false); @@ -142,17 +154,17 @@ public class RecentMailboxManagerTest extends ProviderTestCase2 { assertEquals(2, testList.size()); assertEquals(mMailboxArray[8].mId, (long) testList.get(0)); assertEquals(mMailboxArray[7].mId, (long) testList.get(1)); + } - mManager.touch(mMailboxArray[0].mId); // inbox - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[1].mId); // drafts - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[2].mId); // outbox - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[3].mId); // sent - mMockClock.advance(1000L); - mManager.touch(mMailboxArray[4].mId).get(); // trash - mMockClock.advance(1000L); + /** Test limit for system mailboxes */ + public void testGetMostRecent04() throws Exception { + ArrayList testList; + + mMockClock.advance(1000L); mManager.touch(mMailboxArray[0].mId); // inbox + mMockClock.advance(1000L); mManager.touch(mMailboxArray[1].mId); // drafts + mMockClock.advance(1000L); mManager.touch(mMailboxArray[2].mId); // outbox + mMockClock.advance(1000L); mManager.touch(mMailboxArray[3].mId); // sent + mMockClock.advance(1000L); mManager.touch(mMailboxArray[4].mId).get(); // trash // nothing but system mailboxes testList = mManager.getMostRecent(1L, false); @@ -165,4 +177,37 @@ public class RecentMailboxManagerTest extends ProviderTestCase2 { testList = mManager.getMostRecent(1L, true); assertEquals(0, testList.size()); } + + /** Test limit for user mailboxes */ + public void testGetMostRecent05() throws Exception { + ArrayList testList; + + // test limit for the filtered list + mMockClock.advance(1000L); mManager.touch(mMailboxArray[6].mId); // abbott + mMockClock.advance(1000L); mManager.touch(mMailboxArray[7].mId); // costello + mMockClock.advance(1000L); mManager.touch(mMailboxArray[8].mId); // bud_lou + mMockClock.advance(1000L); mManager.touch(mMailboxArray[9].mId); // laurel + mMockClock.advance(1000L); mManager.touch(mMailboxArray[10].mId); // hardy + mMockClock.advance(1000L); mManager.touch(mMailboxArray[0].mId); // inbox + mMockClock.advance(1000L); mManager.touch(mMailboxArray[1].mId); // drafts + mMockClock.advance(1000L); mManager.touch(mMailboxArray[2].mId); // outbox + mMockClock.advance(1000L); mManager.touch(mMailboxArray[3].mId); // sent + mMockClock.advance(1000L); mManager.touch(mMailboxArray[4].mId).get(); // trash + + // nothing but user mailboxes + testList = mManager.getMostRecent(1L, false); + assertEquals(5, testList.size()); + assertEquals(mMailboxArray[1].mId, (long) testList.get(0)); + assertEquals(mMailboxArray[0].mId, (long) testList.get(1)); + assertEquals(mMailboxArray[2].mId, (long) testList.get(2)); + assertEquals(mMailboxArray[3].mId, (long) testList.get(3)); + assertEquals(mMailboxArray[4].mId, (long) testList.get(4)); + testList = mManager.getMostRecent(1L, true); + assertEquals(5, testList.size()); + assertEquals(mMailboxArray[6].mId, (long) testList.get(0)); + assertEquals(mMailboxArray[8].mId, (long) testList.get(1)); + assertEquals(mMailboxArray[7].mId, (long) testList.get(2)); + assertEquals(mMailboxArray[10].mId, (long) testList.get(3)); + assertEquals(mMailboxArray[9].mId, (long) testList.get(4)); + } }