diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 00060f40c..0153d6eb7 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -297,7 +297,7 @@ public class MessageList extends K9FragmentActivity implements MessageListFragme return true; } case KeyEvent.KEYCODE_G: { - mMessageListFragment.onToggleFlag(); + mMessageListFragment.onToggleFlagged(); return true; } case KeyEvent.KEYCODE_M: { diff --git a/src/com/fsck/k9/controller/MessagingController.java b/src/com/fsck/k9/controller/MessagingController.java index d23d5799c..4d59531d5 100644 --- a/src/com/fsck/k9/controller/MessagingController.java +++ b/src/com/fsck/k9/controller/MessagingController.java @@ -14,6 +14,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicBoolean; @@ -2559,39 +2560,65 @@ public class MessagingController implements Runnable { processPendingCommands(account); } - public void setFlag(final List messages, final Flag flag, final boolean newState) { + public void setFlag(final Account account, final List messageIds, final Flag flag, + final boolean newState, final boolean threadedList) { - actOnMessages(messages, new MessageActor() { + threadPool.execute(new Runnable() { @Override - public void act(final Account account, final Folder folder, - final List accountMessages) { - - setFlag(account, folder.getName(), accountMessages.toArray(EMPTY_MESSAGE_ARRAY), flag, - newState); + public void run() { + setFlagSynchronous(account, messageIds, flag, newState, threadedList); } }); } - public void setFlagForThreads(final List messages, final Flag flag, - final boolean newState) { + private void setFlagSynchronous(final Account account, final List messageIds, + final Flag flag, final boolean newState, final boolean threadedList) { - actOnMessages(messages, new MessageActor() { - @Override - public void act(final Account account, final Folder folder, - final List accountMessages) { + LocalStore localStore; + try { + localStore = account.getLocalStore(); + } catch (MessagingException e) { + Log.e(K9.LOG_TAG, "Couldn't get LocalStore instance", e); + return; + } - try { - List messagesInThreads = collectMessagesInThreads(account, - accountMessages); + // Update affected messages in the database. This should be as fast as possible so the UI + // can be updated with the new state. + try { + localStore.setFlag(messageIds, flag, newState, threadedList); + } catch (MessagingException e) { + Log.e(K9.LOG_TAG, "Couldn't set flags in local database", e); + } - setFlag(account, folder.getName(), - messagesInThreads.toArray(EMPTY_MESSAGE_ARRAY), flag, newState); + // Notify listeners of changed folder status +// for (MessagingListener l : getListeners()) { +// l.folderStatusChanged(account, folderName, localFolder.getUnreadMessageCount()); +// } - } catch (MessagingException e) { - addErrorMessage(account, "Something went wrong in setFlagForThreads()", e); - } + // Read folder name and UID of messages from the database + Map> folderMap; + try { + folderMap = localStore.getFoldersAndUids(messageIds, threadedList); + } catch (MessagingException e) { + Log.e(K9.LOG_TAG, "Couldn't get folder name and UID of messages", e); + return; + } + + // Loop over all folders + for (Entry> entry : folderMap.entrySet()) { + String folderName = entry.getKey(); + + // The error folder is always a local folder + // TODO: Skip the remote part for all local-only folders + if (account.getErrorFolderName().equals(folderName)) { + continue; } - }); + + // Send flag change to server + String[] uids = entry.getValue().toArray(EMPTY_STRING_ARRAY); + queueSetFlag(account, folderName, Boolean.toString(newState), flag.toString(), uids); + processPendingCommands(account); + } } /** @@ -2848,15 +2875,18 @@ public class MessagingController implements Runnable { false, true)) { if (account.isMarkMessageAsReadOnView() && !message.isSet(Flag.SEEN)) { message.setFlag(Flag.SEEN, true); - setFlag(Collections.singletonList((Message) message), - Flag.SEEN, true); + + setFlagSynchronous(account, Collections.singletonList(Long.valueOf(message.getId())), Flag.SEEN, true, false); +// setFlag(Collections.singletonList((Message) message), +// Flag.SEEN, true); } } return; } if (!message.isSet(Flag.SEEN)) { message.setFlag(Flag.SEEN, true); - setFlag(Collections.singletonList((Message) message), Flag.SEEN, true); + setFlagSynchronous(account, Collections.singletonList(Long.valueOf(message.getId())), Flag.SEEN, true, false); +// setFlag(Collections.singletonList((Message) message), Flag.SEEN, true); } for (MessagingListener l : getListeners(listener)) { diff --git a/src/com/fsck/k9/fragment/MessageListFragment.java b/src/com/fsck/k9/fragment/MessageListFragment.java index 8c5462fa4..9bd081b9a 100644 --- a/src/com/fsck/k9/fragment/MessageListFragment.java +++ b/src/com/fsck/k9/fragment/MessageListFragment.java @@ -1257,22 +1257,25 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick public boolean onContextItemSelected(android.view.MenuItem item) { AdapterContextMenuInfo info = (AdapterContextMenuInfo) item.getMenuInfo(); int adapterPosition = listViewToAdapterPosition(info.position); - Message message = getMessageAtPosition(adapterPosition); switch (item.getItemId()) { case R.id.reply: { + Message message = getMessageAtPosition(adapterPosition); onReply(message); break; } case R.id.reply_all: { + Message message = getMessageAtPosition(adapterPosition); onReplyAll(message); break; } case R.id.forward: { + Message message = getMessageAtPosition(adapterPosition); onForward(message); break; } case R.id.send_again: { + Message message = getMessageAtPosition(adapterPosition); onResendMessage(message); mSelectedCount = 0; break; @@ -1286,40 +1289,45 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick break; } case R.id.delete: { + Message message = getMessageAtPosition(adapterPosition); onDelete(message); break; } case R.id.mark_as_read: { - setFlag(message, Flag.SEEN, true); + setFlag(adapterPosition, Flag.SEEN, true); break; } case R.id.mark_as_unread: { - setFlag(message, Flag.SEEN, false); + setFlag(adapterPosition, Flag.SEEN, false); break; } case R.id.flag: { - setFlag(message, Flag.FLAGGED, true); + setFlag(adapterPosition, Flag.FLAGGED, true); break; } case R.id.unflag: { - setFlag(message, Flag.FLAGGED, false); + setFlag(adapterPosition, Flag.FLAGGED, false); break; } // only if the account supports this case R.id.archive: { + Message message = getMessageAtPosition(adapterPosition); onArchive(message); break; } case R.id.spam: { + Message message = getMessageAtPosition(adapterPosition); onSpam(message); break; } case R.id.move: { + Message message = getMessageAtPosition(adapterPosition); onMove(message); break; } case R.id.copy: { + Message message = getMessageAtPosition(adapterPosition); onCopy(message); break; } @@ -1968,19 +1976,59 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick mActionModeCallback.showFlag(isBatchFlag); } - private void setFlag(Message message, final Flag flag, final boolean newState) { - setFlag(Collections.singletonList(message), flag, newState); - } - - private void setFlag(List messages, final Flag flag, final boolean newState) { - if (messages.size() == 0) { + private void setFlag(int adapterPosition, final Flag flag, final boolean newState) { + if (adapterPosition == AdapterView.INVALID_POSITION) { return; } - if (mThreadedList) { - mController.setFlagForThreads(messages, flag, newState); - } else { - mController.setFlag(messages, flag, newState); + Cursor cursor = (Cursor) mAdapter.getItem(adapterPosition); + Account account = mPreferences.getAccount(cursor.getString(ACCOUNT_UUID_COLUMN)); + long id = cursor.getLong(ID_COLUMN); + + mController.setFlag(account, Collections.singletonList(Long.valueOf(id)), flag, newState, + mThreadedList); + + computeBatchDirection(); + } + + private void setFlagForSelected(final Flag flag, final boolean newState) { + if (mSelected.size() == 0) { + return; + } + + Map> accountMapping = new HashMap>(); + Set accounts = new HashSet(); + + for (int position = 0, end = mAdapter.getCount(); position < end; position++) { + Cursor cursor = (Cursor) mAdapter.getItem(position); + long uniqueId = cursor.getLong(mUniqueIdColumn); + + if (mSelected.contains(uniqueId)) { + String uuid = cursor.getString(ACCOUNT_UUID_COLUMN); + Account account = mPreferences.getAccount(uuid); + + accounts.add(account); + List messageIdList = accountMapping.get(account); + if (messageIdList == null) { + messageIdList = new ArrayList(); + accountMapping.put(account, messageIdList); + } + + long selectionId; + if (mThreadedList) { + selectionId = (cursor.isNull(THREAD_ROOT_COLUMN)) ? + cursor.getLong(ID_COLUMN) : cursor.getLong(THREAD_ROOT_COLUMN); + } else { + selectionId = cursor.getLong(ID_COLUMN); + } + + messageIdList.add(selectionId); + } + } + + for (Account account : accounts) { + List messageIds = accountMapping.get(account); + mController.setFlag(account, messageIds, flag, newState, mThreadedList); } computeBatchDirection(); @@ -2408,8 +2456,6 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick @Override public boolean onActionItemClicked(ActionMode mode, MenuItem item) { - List messages = getCheckedMessages(); - /* * In the following we assume that we can't move or copy * mails to the same folder. Also that spam isn't available if we are @@ -2419,26 +2465,25 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick */ switch (item.getItemId()) { case R.id.delete: { + List messages = getCheckedMessages(); onDelete(messages); - - //FIXME mSelectedCount = 0; break; } case R.id.mark_as_read: { - setFlag(messages, Flag.SEEN, true); + setFlagForSelected(Flag.SEEN, true); break; } case R.id.mark_as_unread: { - setFlag(messages, Flag.SEEN, false); + setFlagForSelected(Flag.SEEN, false); break; } case R.id.flag: { - setFlag(messages, Flag.FLAGGED, true); + setFlagForSelected(Flag.FLAGGED, true); break; } case R.id.unflag: { - setFlag(messages, Flag.FLAGGED, false); + setFlagForSelected(Flag.FLAGGED, false); break; } case R.id.select_all: { @@ -2448,21 +2493,25 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick // only if the account supports this case R.id.archive: { + List messages = getCheckedMessages(); onArchive(messages); mSelectedCount = 0; break; } case R.id.spam: { + List messages = getCheckedMessages(); onSpam(messages); mSelectedCount = 0; break; } case R.id.move: { + List messages = getCheckedMessages(); onMove(messages); mSelectedCount = 0; break; } case R.id.copy: { + List messages = getCheckedMessages(); onCopy(messages); mSelectedCount = 0; break; @@ -2608,6 +2657,11 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick return getMessageAtPosition(adapterPosition); } + private int getAdapterPositionForSelectedMessage() { + int listViewPosition = mListView.getSelectedItemPosition(); + return listViewToAdapterPosition(listViewPosition); + } + private Message getMessageAtPosition(int adapterPosition) { if (adapterPosition == AdapterView.INVALID_POSITION) { return null; @@ -2654,11 +2708,23 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick toggleMessageSelect(mListView.getSelectedItemPosition()); } - public void onToggleFlag() { - Message message = getSelectedMessage(); - if (message != null) { - setFlag(message, Flag.FLAGGED, !message.isSet(Flag.FLAGGED)); + public void onToggleFlagged() { + onToggleFlag(Flag.FLAGGED, FLAGGED_COLUMN); + } + + public void onToggleRead() { + onToggleFlag(Flag.SEEN, READ_COLUMN); + } + + private void onToggleFlag(Flag flag, int flagColumn) { + int adapterPosition = getAdapterPositionForSelectedMessage(); + if (adapterPosition == ListView.INVALID_POSITION) { + return; } + + Cursor cursor = (Cursor) mAdapter.getItem(adapterPosition); + boolean flagState = (cursor.getInt(flagColumn) == 1); + setFlag(adapterPosition, flag, !flagState); } public void onMove() { @@ -2682,13 +2748,6 @@ public class MessageListFragment extends SherlockFragment implements OnItemClick } } - public void onToggleRead() { - Message message = getSelectedMessage(); - if (message != null) { - setFlag(message, Flag.SEEN, !message.isSet(Flag.SEEN)); - } - } - public boolean isOutbox() { return (mFolderName != null && mFolderName.equals(mAccount.getOutboxFolderName())); } diff --git a/src/com/fsck/k9/mail/store/LocalStore.java b/src/com/fsck/k9/mail/store/LocalStore.java index 34c12ffe1..cde4941ac 100644 --- a/src/com/fsck/k9/mail/store/LocalStore.java +++ b/src/com/fsck/k9/mail/store/LocalStore.java @@ -111,6 +111,13 @@ public class LocalStore extends Store implements Serializable { */ private static final int UID_CHECK_BATCH_SIZE = 500; + /** + * Number of messages to perform flag updates at once. + * + * @see #setFlag(List, Flag, boolean, boolean) + */ + private static final int FLAG_UPDATE_BATCH_SIZE = 500; + public static final int DB_VERSION = 46; protected String uUid = null; @@ -4092,4 +4099,273 @@ public class LocalStore extends Store implements Serializable { Uri uri = Uri.withAppendedPath(EmailProvider.CONTENT_URI, "account/" + uUid + "/messages"); mContentResolver.notifyChange(uri, null); } + + /** + * Split database operations with a large set of arguments into multiple SQL statements. + * + *

+ * At the time of this writing (2012-12-06) SQLite only supports around 1000 arguments. That's + * why we have to split SQL statements with a large set of arguments into multiple SQL + * statements each working on a subset of the arguments. + *

+ * + * @param selectionCallback + * Supplies the argument set and the code to query/update the database. + * @param batchSize + * The maximum size of the selection set in each SQL statement. + * + * @throws MessagingException + */ + public void doBatchSetSelection(final BatchSetSelection selectionCallback, final int batchSize) + throws MessagingException { + + final List selectionArgs = new ArrayList(); + int start = 0; + + while (start < selectionCallback.getListSize()) { + final StringBuilder selection = new StringBuilder(); + + selection.append(" IN ("); + + int count = Math.min(selectionCallback.getListSize() - start, batchSize); + + for (int i = start, end = start + count; i < end; i++) { + if (i > start) { + selection.append(",?"); + } else { + selection.append("?"); + } + + selectionArgs.add(selectionCallback.getListItem(i)); + } + + selection.append(")"); + + try { + database.execute(true, new DbCallback() { + @Override + public Void doDbWork(final SQLiteDatabase db) throws WrappedException, + UnavailableStorageException { + + selectionCallback.doDbWork(db, selection.toString(), + selectionArgs.toArray(EMPTY_STRING_ARRAY)); + + return null; + } + }); + + selectionCallback.postDbWork(); + + } catch (WrappedException e) { + throw(MessagingException) e.getCause(); + } + + selectionArgs.clear(); + start += count; + } + } + + /** + * Defines the behavior of {@link LocalStore#doBatchSetSelection(BatchSetSelection, int)}. + */ + public interface BatchSetSelection { + /** + * @return The size of the argument list. + */ + int getListSize(); + + /** + * Get a specific item of the argument list. + * + * @param index + * The index of the item. + * + * @return Item at position {@code i} of the argument list. + */ + String getListItem(int index); + + /** + * Execute the SQL statement. + * + * @param db + * Use this {@link SQLiteDatabase} instance for your SQL statement. + * @param selectionSet + * A partial selection string containing place holders for the argument list, e.g. + * {@code " IN (?,?,?)"} (starts with a space). + * @param selectionArgs + * The current subset of the argument list. + * @throws UnavailableStorageException + */ + void doDbWork(SQLiteDatabase db, String selectionSet, String[] selectionArgs) + throws UnavailableStorageException; + + /** + * This will be executed after each invocation of + * {@link #doDbWork(SQLiteDatabase, String, String[])} (after the transaction has been + * committed). + */ + void postDbWork(); + } + + /** + * Change the state of a flag for a list of messages. + * + *

+ * The goal of this method is to be fast. Currently this means using as few SQL UPDATE + * statements as possible.
+ * Current benchmarks show that updating 1000 messages takes about 8 seconds on a Nexus 7. So + * there should be room for further improvement. + *

+ * + * @param messageIds + * A list of primary keys in the "messages" table. + * @param flag + * The flag to change. This must be a flag with a separate column in the database. + * @param newState + * {@code true}, if the flag should be set. {@code false}, otherwise. + * @param threadRootIds + * If this is {@code true}, {@code messageIds} contains the IDs of the messages at the + * root of a thread. In that case the flag is changed for all messages in these threads. + * If this is {@code false} only the messages in {@code messageIds} are changed. + * + * @throws MessagingException + */ + public void setFlag(final List messageIds, final Flag flag, + final boolean newState, final boolean threadRootIds) throws MessagingException { + + final ContentValues cv = new ContentValues(); + + switch (flag) { + case SEEN: { + cv.put("read", newState); + break; + } + case FLAGGED: { + cv.put("flagged", newState); + break; + } + case ANSWERED: { + cv.put("answered", newState); + break; + } + case FORWARDED: { + cv.put("forwarded", newState); + break; + } + default: { + throw new IllegalArgumentException("Flag must be a special column flag"); + } + } + + doBatchSetSelection(new BatchSetSelection() { + + @Override + public int getListSize() { + return messageIds.size(); + } + + @Override + public String getListItem(int index) { + return Long.toString(messageIds.get(index)); + } + + @Override + public void doDbWork(SQLiteDatabase db, String selectionSet, String[] selectionArgs) + throws UnavailableStorageException { + + db.update("messages", cv, "(empty IS NULL OR empty != 1) AND id" + selectionSet, + selectionArgs); + + if (threadRootIds) { + db.update("messages", cv, "(empty IS NULL OR empty != 1) AND thread_root" + + selectionSet, selectionArgs); + } + } + + @Override + public void postDbWork() { + notifyChange(); + } + }, FLAG_UPDATE_BATCH_SIZE); + } + + /** + * Get folder name and UID for the supplied messages. + * + * @param messageIds + * A list of primary keys in the "messages" table. + * @param threadedList + * If this is {@code true}, {@code messageIds} contains the IDs of the messages at the + * root of a thread. In that case return UIDs for all messages in these threads. + * If this is {@code false} only the UIDs for messages in {@code messageIds} are + * returned. + * + * @return The list of UIDs for the messages grouped by folder name. + * + * @throws MessagingException + */ + public Map> getFoldersAndUids(final List messageIds, + final boolean threadedList) throws MessagingException { + + final Map> folderMap = new HashMap>(); + + doBatchSetSelection(new BatchSetSelection() { + + @Override + public int getListSize() { + return messageIds.size(); + } + + @Override + public String getListItem(int index) { + return Long.toString(messageIds.get(index)); + } + + @Override + public void doDbWork(SQLiteDatabase db, String selectionSet, String[] selectionArgs) + throws UnavailableStorageException { + + String sqlPrefix = + "SELECT m.uid, f.name " + + "FROM messages m " + + "JOIN folders f ON (m.folder_id = f.id) " + + "WHERE (m.empty IS NULL OR m.empty != 1) AND "; + + String sql = sqlPrefix + "m.id" + selectionSet; + getDataFromCursor(db.rawQuery(sql, selectionArgs)); + + if (threadedList) { + String threadSql = sqlPrefix + "m.thread_root" + selectionSet; + getDataFromCursor(db.rawQuery(threadSql, selectionArgs)); + } + } + + private void getDataFromCursor(Cursor cursor) { + try { + while (cursor.moveToNext()) { + String uid = cursor.getString(0); + String folderName = cursor.getString(1); + + List uidList = folderMap.get(folderName); + if (uidList == null) { + uidList = new ArrayList(); + folderMap.put(folderName, uidList); + } + + uidList.add(uid); + } + } finally { + cursor.close(); + } + } + + @Override + public void postDbWork() { + notifyChange(); + + } + }, UID_CHECK_BATCH_SIZE); + + return folderMap; + } }