From faa666394ce0bf9e4239869ee194991d2f3308f1 Mon Sep 17 00:00:00 2001 From: cketti Date: Sat, 27 Oct 2012 02:15:30 +0200 Subject: [PATCH] Do sorting in MergeCursor when merging the query results Disabled "sort by sender" for now because the database can't sort by contact names from the contacts database. We probably have to special-case that and do in-memory sorting. --- res/menu/message_list_option.xml | 2 + src/com/fsck/k9/Account.java | 2 +- src/com/fsck/k9/activity/MessageList.java | 8 +- .../fsck/k9/fragment/MessageListFragment.java | 250 ++++++++++-------- src/com/fsck/k9/helper/MergeCursor.java | 157 ++++++++--- .../k9/helper/MergeCursorWithUniqueId.java | 6 +- 6 files changed, 270 insertions(+), 155 deletions(-) diff --git a/res/menu/message_list_option.xml b/res/menu/message_list_option.xml index 106dfab5a..ed0a09078 100644 --- a/res/menu/message_list_option.xml +++ b/res/menu/message_list_option.xml @@ -29,9 +29,11 @@ + diff --git a/src/com/fsck/k9/Account.java b/src/com/fsck/k9/Account.java index 6fe63d3ba..5ce0d3f96 100644 --- a/src/com/fsck/k9/Account.java +++ b/src/com/fsck/k9/Account.java @@ -94,7 +94,7 @@ public class Account implements BaseAccount { SORT_DATE(R.string.sort_earliest_first, R.string.sort_latest_first, false), SORT_ARRIVAL(R.string.sort_earliest_first, R.string.sort_latest_first, false), SORT_SUBJECT(R.string.sort_subject_alpha, R.string.sort_subject_re_alpha, true), - SORT_SENDER(R.string.sort_sender_alpha, R.string.sort_sender_re_alpha, true), +// SORT_SENDER(R.string.sort_sender_alpha, R.string.sort_sender_re_alpha, true), SORT_UNREAD(R.string.sort_unread_first, R.string.sort_unread_last, true), SORT_FLAGGED(R.string.sort_flagged_first, R.string.sort_flagged_last, true), SORT_ATTACHMENT(R.string.sort_attach_first, R.string.sort_unattached_first, true); diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 92ab7d2e4..bf8f296a9 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -374,10 +374,10 @@ public class MessageList extends K9FragmentActivity implements MessageListFragme mMessageListFragment.changeSort(SortType.SORT_SUBJECT); return true; } - case R.id.set_sort_sender: { - mMessageListFragment.changeSort(SortType.SORT_SENDER); - return true; - } +// case R.id.set_sort_sender: { +// mMessageListFragment.changeSort(SortType.SORT_SENDER); +// return true; +// } case R.id.set_sort_flag: { mMessageListFragment.changeSort(SortType.SORT_FLAGGED); return true; diff --git a/src/com/fsck/k9/fragment/MessageListFragment.java b/src/com/fsck/k9/fragment/MessageListFragment.java index c58fd1df7..9b0dd87e0 100644 --- a/src/com/fsck/k9/fragment/MessageListFragment.java +++ b/src/com/fsck/k9/fragment/MessageListFragment.java @@ -7,8 +7,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.Date; import java.util.EnumMap; -import java.util.HashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.concurrent.Future; @@ -69,7 +67,6 @@ import com.fsck.k9.R; import com.fsck.k9.activity.ActivityListener; import com.fsck.k9.activity.ChooseFolder; import com.fsck.k9.activity.FolderInfoHolder; -import com.fsck.k9.activity.MessageInfoHolder; import com.fsck.k9.activity.MessageReference; import com.fsck.k9.controller.MessagingController; import com.fsck.k9.fragment.ConfirmationDialogFragment; @@ -77,7 +74,6 @@ import com.fsck.k9.fragment.ConfirmationDialogFragment.ConfirmationDialogFragmen import com.fsck.k9.helper.MessageHelper; import com.fsck.k9.helper.MergeCursorWithUniqueId; import com.fsck.k9.helper.StringUtils; -import com.fsck.k9.helper.Utility; import com.fsck.k9.mail.Address; import com.fsck.k9.mail.Flag; import com.fsck.k9.mail.Folder; @@ -173,7 +169,6 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick // arg1 & 2 are mixed up, this is done on purpose return mDelegate.compare(object2, object1); } - } /** @@ -182,7 +177,6 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick * @param */ public static class ComparatorChain implements Comparator { - private List> mChain; /** @@ -204,89 +198,93 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick } return result; } - } - public static class AttachmentComparator implements Comparator { + public static class ReverseIdComparator implements Comparator { + private int mIdColumn = -1; @Override - public int compare(MessageInfoHolder object1, MessageInfoHolder object2) { - return (object1.message.hasAttachments() ? 0 : 1) - (object2.message.hasAttachments() ? 0 : 1); + public int compare(Cursor cursor1, Cursor cursor2) { + if (mIdColumn == -1) { + mIdColumn = cursor1.getColumnIndex("_id"); + } + long o1Id = cursor1.getLong(mIdColumn); + long o2Id = cursor2.getLong(mIdColumn); + return (o1Id > o2Id) ? -1 : 1; } - } - public static class FlaggedComparator implements Comparator { + public static class AttachmentComparator implements Comparator { @Override - public int compare(MessageInfoHolder object1, MessageInfoHolder object2) { - return (object1.flagged ? 0 : 1) - (object2.flagged ? 0 : 1); + public int compare(Cursor cursor1, Cursor cursor2) { + int o1HasAttachment = (cursor1.getInt(ATTACHMENT_COUNT_COLUMN) > 0) ? 0 : 1; + int o2HasAttachment = (cursor2.getInt(ATTACHMENT_COUNT_COLUMN) > 0) ? 0 : 1; + return o1HasAttachment - o2HasAttachment; } - } - public static class UnreadComparator implements Comparator { + public static class FlaggedComparator implements Comparator { @Override - public int compare(MessageInfoHolder object1, MessageInfoHolder object2) { - return (object1.read ? 1 : 0) - (object2.read ? 1 : 0); + public int compare(Cursor cursor1, Cursor cursor2) { + int o1IsFlagged = (cursor1.getString(FLAGS_COLUMN).contains("FLAGGED")) ? 0 : 1; + int o2IsFlagged = (cursor2.getString(FLAGS_COLUMN).contains("FLAGGED")) ? 0 : 1; + return o1IsFlagged - o2IsFlagged; } - } - public static class SenderComparator implements Comparator { + public static class UnreadComparator implements Comparator { @Override - public int compare(MessageInfoHolder object1, MessageInfoHolder object2) { - if (object1.compareCounterparty == null) { - return (object2.compareCounterparty == null ? 0 : 1); - } else if (object2.compareCounterparty == null) { + public int compare(Cursor cursor1, Cursor cursor2) { + int o1IsUnread = (cursor1.getString(FLAGS_COLUMN).contains("SEEN")) ? 1 : 0; + int o2IsUnread = (cursor2.getString(FLAGS_COLUMN).contains("SEEN")) ? 1 : 0; + return o1IsUnread - o2IsUnread; + } + } + + public static class DateComparator implements Comparator { + + @Override + public int compare(Cursor cursor1, Cursor cursor2) { + long o1Date = cursor1.getLong(DATE_COLUMN); + long o2Date = cursor2.getLong(DATE_COLUMN); + if (o1Date < o2Date) { + return -1; + } else if (o1Date == o2Date) { + return 0; + } else { + return 1; + } + } + } + + public static class ArrivalComparator implements Comparator { + + @Override + public int compare(Cursor cursor1, Cursor cursor2) { + long o1Date = cursor1.getLong(INTERNAL_DATE_COLUMN); + long o2Date = cursor2.getLong(INTERNAL_DATE_COLUMN); + if (o1Date == o2Date) { + return 0; + } else if (o1Date < o2Date) { return -1; } else { - return object1.compareCounterparty.toLowerCase().compareTo(object2.compareCounterparty.toLowerCase()); + return 1; } } - } - public static class DateComparator implements Comparator { + public static class SubjectComparator implements Comparator { @Override - public int compare(MessageInfoHolder object1, MessageInfoHolder object2) { - if (object1.compareDate == null) { - return (object2.compareDate == null ? 0 : 1); - } else if (object2.compareDate == null) { - return -1; - } else { - return object1.compareDate.compareTo(object2.compareDate); - } + public int compare(Cursor cursor1, Cursor cursor2) { + String subject1 = cursor1.getString(SUBJECT_COLUMN); + String subject2 = cursor2.getString(SUBJECT_COLUMN); + + return subject1.compareToIgnoreCase(subject2); } - - } - - public static class ArrivalComparator implements Comparator { - - @Override - public int compare(MessageInfoHolder object1, MessageInfoHolder object2) { - return object1.compareArrival.compareTo(object2.compareArrival); - } - - } - - public static class SubjectComparator implements Comparator { - - @Override - public int compare(MessageInfoHolder arg0, MessageInfoHolder arg1) { - // XXX doesn't respect the Comparator contract since it alters the compared object - if (arg0.compareSubject == null) { - arg0.compareSubject = Utility.stripSubject(arg0.message.getSubject()); - } - if (arg1.compareSubject == null) { - arg1.compareSubject = Utility.stripSubject(arg1.message.getSubject()); - } - return arg0.compareSubject.compareToIgnoreCase(arg1.compareSubject); - } - } @@ -301,17 +299,17 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick /** * Maps a {@link SortType} to a {@link Comparator} implementation. */ - private static final Map> SORT_COMPARATORS; + private static final Map> SORT_COMPARATORS; static { // fill the mapping at class time loading - final Map> map = new EnumMap>(SortType.class); + final Map> map = + new EnumMap>(SortType.class); map.put(SortType.SORT_ATTACHMENT, new AttachmentComparator()); map.put(SortType.SORT_DATE, new DateComparator()); map.put(SortType.SORT_ARRIVAL, new ArrivalComparator()); map.put(SortType.SORT_FLAGGED, new FlaggedComparator()); - map.put(SortType.SORT_SENDER, new SenderComparator()); map.put(SortType.SORT_SUBJECT, new SubjectComparator()); map.put(SortType.SORT_UNREAD, new UnreadComparator()); @@ -471,35 +469,33 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick * @return The comparator to use to display messages in an ordered * fashion. Never null. */ - protected Comparator getComparator() { - final List> chain = new ArrayList>(2 /* we add 2 comparators at most */); + protected Comparator getComparator() { + final List> chain = + new ArrayList>(3 /* we add 3 comparators at most */); - { - // add the specified comparator - final Comparator comparator = SORT_COMPARATORS.get(mSortType); - if (mSortAscending) { - chain.add(comparator); + // Add the specified comparator + final Comparator comparator = SORT_COMPARATORS.get(mSortType); + if (mSortAscending) { + chain.add(comparator); + } else { + chain.add(new ReverseComparator(comparator)); + } + + // Add the date comparator if not already specified + if (mSortType != SortType.SORT_DATE && mSortType != SortType.SORT_ARRIVAL) { + final Comparator dateComparator = SORT_COMPARATORS.get(SortType.SORT_DATE); + if (mSortDateAscending) { + chain.add(dateComparator); } else { - chain.add(new ReverseComparator(comparator)); + chain.add(new ReverseComparator(dateComparator)); } } - { - // add the date comparator if not already specified - if (mSortType != SortType.SORT_DATE && mSortType != SortType.SORT_ARRIVAL) { - final Comparator comparator = SORT_COMPARATORS.get(SortType.SORT_DATE); - if (mSortDateAscending) { - chain.add(comparator); - } else { - chain.add(new ReverseComparator(comparator)); - } - } - } + // Add the id comparator + chain.add(new ReverseIdComparator()); - // build the comparator chain - final Comparator chainComparator = new ComparatorChain(chain); - - return chainComparator; + // Build the comparator chain + return new ComparatorChain(chain); } private void folderLoading(String folder, boolean loading) { @@ -699,6 +695,9 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick initializeMessageList(); + // This needs to be done before initializing the cursor loader below + initializeSortSettings(); + LoaderManager loaderManager = getLoaderManager(); int len = mAccountUuids.length; mCursors = new Cursor[len]; @@ -707,6 +706,18 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick } } + private void initializeSortSettings() { + if (mSingleAccountMode) { + mSortType = mAccount.getSortType(); + mSortAscending = mAccount.isSortAscending(mSortType); + mSortDateAscending = mAccount.isSortAscending(SortType.SORT_DATE); + } else { + mSortType = K9.getSortType(); + mSortAscending = K9.isSortAscending(mSortType); + mSortDateAscending = K9.isSortAscending(SortType.SORT_DATE); + } + } + private void decodeArguments() { Bundle args = getArguments(); @@ -781,11 +792,24 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick } private String getFolderNameById(Account account, long folderId) { + try { + Folder folder = getFolderById(account, folderId); + if (folder != null) { + return folder.getName(); + } + } catch (Exception e) { + Log.e(K9.LOG_TAG, "getFolderNameById() failed.", e); + } + + return null; + } + + private Folder getFolderById(Account account, long folderId) { try { LocalStore localStore = account.getLocalStore(); LocalFolder localFolder = localStore.getFolderById(folderId); localFolder.open(OpenMode.READ_ONLY); - return localFolder.getName(); + return localFolder; } catch (Exception e) { Log.e(K9.LOG_TAG, "getFolderNameById() failed.", e); return null; @@ -879,17 +903,10 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick Account[] accountsWithNotification; Account account = mAccount; - if (account != null) { accountsWithNotification = new Account[] { account }; - mSortType = account.getSortType(); - mSortAscending = account.isSortAscending(mSortType); - mSortDateAscending = account.isSortAscending(SortType.SORT_DATE); } else { accountsWithNotification = mPreferences.getAccounts(); - mSortType = K9.getSortType(); - mSortAscending = K9.isSortAscending(mSortType); - mSortDateAscending = K9.isSortAscending(SortType.SORT_DATE); } for (Account accountWithNotification : accountsWithNotification) { @@ -1145,10 +1162,10 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick changeSort(SortType.SORT_SUBJECT); return true; } - case R.id.set_sort_sender: { - changeSort(SortType.SORT_SENDER); - return true; - } +// case R.id.set_sort_sender: { +// changeSort(SortType.SORT_SENDER); +// return true; +// } case R.id.set_sort_flag: { changeSort(SortType.SORT_FLAGGED); return true; @@ -2496,8 +2513,9 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick Cursor cursor = (Cursor) mAdapter.getItem(adapterPosition); String uid = cursor.getString(UID_COLUMN); - //TODO: get account and folder from cursor - Folder folder = mCurrentFolder.folder; + Account account = getAccountFromCursor(cursor); + long folderId = cursor.getLong(FOLDER_ID_COLUMN); + Folder folder = getFolderById(account, folderId); try { return folder.getMessage(uid); @@ -2668,13 +2686,13 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick sortColumn = "(" + MessageColumns.FLAGS + " NOT LIKE '%FLAGGED%')"; break; } - case SORT_SENDER: { - //FIXME - sortColumn = MessageColumns.SENDER_LIST; - break; - } +// case SORT_SENDER: { +// //FIXME +// sortColumn = MessageColumns.SENDER_LIST; +// break; +// } case SORT_SUBJECT: { - sortColumn = MessageColumns.SUBJECT; + sortColumn = MessageColumns.SUBJECT + " COLLATE NOCASE"; break; } case SORT_UNREAD: { @@ -2687,14 +2705,12 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick } } - String sortDirection; + String sortDirection = (mSortAscending) ? " ASC" : " DESC"; String secondarySort; - if (mSortType == SortType.SORT_DATE) { - sortDirection = (mSortDateAscending) ? " ASC" : " DESC"; + if (mSortType == SortType.SORT_DATE || mSortType == SortType.SORT_ARRIVAL) { secondarySort = ""; } else { - sortDirection = (mSortAscending) ? " ASC" : " DESC"; - secondarySort = MessageColumns.DATE + " DESC, "; + secondarySort = MessageColumns.DATE + ((mSortDateAscending) ? " ASC, " : " DESC, "); } String sortOrder = sortColumn + sortDirection + ", " + secondarySort + @@ -2759,9 +2775,13 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick @Override public void onLoadFinished(Loader loader, Cursor data) { - mCursors[loader.getId()] = data; - - MergeCursorWithUniqueId cursor = new MergeCursorWithUniqueId(mCursors); + Cursor cursor; + if (mCursors.length > 1) { + mCursors[loader.getId()] = data; + cursor = new MergeCursorWithUniqueId(mCursors, getComparator()); + } else { + cursor = data; + } mSelected = new SparseBooleanArray(cursor.getCount()); //TODO: use the (stable) IDs as index and reuse the old mSelected diff --git a/src/com/fsck/k9/helper/MergeCursor.java b/src/com/fsck/k9/helper/MergeCursor.java index 1edc6360c..39776e646 100644 --- a/src/com/fsck/k9/helper/MergeCursor.java +++ b/src/com/fsck/k9/helper/MergeCursor.java @@ -17,6 +17,8 @@ package com.fsck.k9.helper; +import java.util.Comparator; + import android.annotation.TargetApi; import android.content.ContentResolver; import android.database.CharArrayBuffer; @@ -48,30 +50,53 @@ public class MergeCursor implements Cursor { */ protected int mActiveCursorIndex; + /** + * The cursor's current position. + */ protected int mPosition; /** - * Used to cache the value of {@link #getCount()} + * Used to cache the value of {@link #getCount()}. */ private int mCount = -1; + /** + * The comparator that is used to decide how the individual cursors are merged. + */ + private final Comparator mComparator; + /** * Constructor * * @param cursors * The list of cursors this {@code MultiCursor} should combine. + * @param comparator + * A comparator that is used to decide in what order the individual cursors are merged. */ - public MergeCursor(Cursor[] cursors) { + public MergeCursor(Cursor[] cursors, Comparator comparator) { mCursors = cursors.clone(); + mComparator = comparator; + + resetCursors(); + } + + private void resetCursors() { + mActiveCursorIndex = -1; + mActiveCursor = null; + mPosition = -1; for (int i = 0, len = mCursors.length; i < len; i++) { - if (mCursors[i] != null) { - mActiveCursorIndex = i; - mActiveCursor = mCursors[mActiveCursorIndex]; + Cursor cursor = mCursors[i]; + if (cursor != null) { + cursor.moveToPosition(-1); + + if (mActiveCursor == null) { + mActiveCursorIndex = i; + mActiveCursor = mCursors[mActiveCursorIndex]; + } } } - mPosition = -1; } @Override @@ -255,7 +280,50 @@ public class MergeCursor implements Cursor { @Override public boolean moveToNext() { - return moveToPosition(mPosition + 1); + int count = getCount(); + if (mPosition == count) { + return false; + } + + if (mPosition == count - 1) { + mActiveCursor.moveToNext(); + mPosition++; + return false; + } + + int smallest = -1; + for (int i = 0, len = mCursors.length; i < len; i++) { + if (mCursors[i] == null || mCursors[i].isLast()) { + continue; + } + + if (smallest == -1) { + smallest = i; + mCursors[smallest].moveToNext(); + continue; + } + + Cursor left = mCursors[smallest]; + Cursor right = mCursors[i]; + + right.moveToNext(); + + int result = mComparator.compare(left, right); + if (result > 0) { + smallest = i; + left.moveToPrevious(); + } else { + right.moveToPrevious(); + } + } + + mPosition++; + if (smallest != -1) { + mActiveCursorIndex = smallest; + mActiveCursor = mCursors[mActiveCursorIndex]; + } + + return true; } @Override @@ -278,40 +346,63 @@ public class MergeCursor implements Cursor { return true; } - /* Find the right cursor */ - mActiveCursor = null; - mActiveCursorIndex = -1; - mPosition = -1; - int cursorStartPos = 0; - - for (int i = 0, len = mCursors.length; i < len; i++) { - if (mCursors[i] == null) { - continue; + if (position > mPosition) { + for (int i = 0, end = position - mPosition; i < end; i++) { + if (!moveToNext()) { + return false; + } } - - if (position < (cursorStartPos + mCursors[i].getCount())) { - mActiveCursorIndex = i; - mActiveCursor = mCursors[mActiveCursorIndex]; - break; + } else { + for (int i = 0, end = mPosition - position; i < end; i++) { + if (!moveToPrevious()) { + return false; + } } - - cursorStartPos += mCursors[i].getCount(); } - /* Move it to the right position */ - if (mActiveCursor != null) { - boolean success = mActiveCursor.moveToPosition(position - cursorStartPos); - mPosition = (success) ? position : -1; - - return success; - } - - return false; + return true; } @Override public boolean moveToPrevious() { - return moveToPosition(mPosition - 1); + if (mPosition < 0) { + return false; + } + + mActiveCursor.moveToPrevious(); + + if (mPosition == 0) { + mPosition = -1; + return false; + } + + int greatest = -1; + for (int i = 0, len = mCursors.length; i < len; i++) { + if (mCursors[i] == null || mCursors[i].isBeforeFirst()) { + continue; + } + + if (greatest == -1) { + greatest = i; + continue; + } + + Cursor left = mCursors[greatest]; + Cursor right = mCursors[i]; + + int result = mComparator.compare(left, right); + if (result <= 0) { + greatest = i; + } + } + + mPosition--; + if (greatest != -1) { + mActiveCursorIndex = greatest; + mActiveCursor = mCursors[mActiveCursorIndex]; + } + + return true; } @Override diff --git a/src/com/fsck/k9/helper/MergeCursorWithUniqueId.java b/src/com/fsck/k9/helper/MergeCursorWithUniqueId.java index 7cad2b084..111d49fbb 100644 --- a/src/com/fsck/k9/helper/MergeCursorWithUniqueId.java +++ b/src/com/fsck/k9/helper/MergeCursorWithUniqueId.java @@ -1,5 +1,7 @@ package com.fsck.k9.helper; +import java.util.Comparator; + import android.database.Cursor; @@ -12,8 +14,8 @@ public class MergeCursorWithUniqueId extends MergeCursor { private int mIdColumnIndex = -1; - public MergeCursorWithUniqueId(Cursor[] cursors) { - super(cursors); + public MergeCursorWithUniqueId(Cursor[] cursors, Comparator comparator) { + super(cursors, comparator); if (cursors.length > MAX_CURSORS) { throw new IllegalArgumentException("This class only supports up to " +