Don't create Message objects when changing flags of selected messages

This changes the interface to MessagingController and the way flags are
updated in the database. Now messages aren't changed one by one but in
batches of 500. This should give better performance, but breaks the
unread and flagged count. I'm not very sad about this, because now we
can move towards only displaying the number of unread/flagged messages
in the local database.
This commit is contained in:
cketti 2012-12-06 05:25:41 +01:00
parent 2b98e1fa8d
commit d5bb462917
4 changed files with 425 additions and 60 deletions

View File

@ -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: {

View File

@ -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<Message> messages, final Flag flag, final boolean newState) {
public void setFlag(final Account account, final List<Long> 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<Message> 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<Message> messages, final Flag flag,
final boolean newState) {
private void setFlagSynchronous(final Account account, final List<Long> 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<Message> accountMessages) {
LocalStore localStore;
try {
localStore = account.getLocalStore();
} catch (MessagingException e) {
Log.e(K9.LOG_TAG, "Couldn't get LocalStore instance", e);
return;
}
try {
List<Message> 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<String, List<String>> 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<String, List<String>> 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)) {

View File

@ -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<Message> 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<Account, List<Long>> accountMapping = new HashMap<Account, List<Long>>();
Set<Account> accounts = new HashSet<Account>();
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<Long> messageIdList = accountMapping.get(account);
if (messageIdList == null) {
messageIdList = new ArrayList<Long>();
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<Long> 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<Message> 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<Message> 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<Message> messages = getCheckedMessages();
onArchive(messages);
mSelectedCount = 0;
break;
}
case R.id.spam: {
List<Message> messages = getCheckedMessages();
onSpam(messages);
mSelectedCount = 0;
break;
}
case R.id.move: {
List<Message> messages = getCheckedMessages();
onMove(messages);
mSelectedCount = 0;
break;
}
case R.id.copy: {
List<Message> 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()));
}

View File

@ -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.
*
* <p>
* 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.
* </p>
*
* @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<String> selectionArgs = new ArrayList<String>();
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<Void>() {
@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.
*
* <p>
* The goal of this method is to be fast. Currently this means using as few SQL UPDATE
* statements as possible.<br>
* Current benchmarks show that updating 1000 messages takes about 8 seconds on a Nexus 7. So
* there should be room for further improvement.
* </p>
*
* @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<Long> 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<String, List<String>> getFoldersAndUids(final List<Long> messageIds,
final boolean threadedList) throws MessagingException {
final Map<String, List<String>> folderMap = new HashMap<String, List<String>>();
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<String> uidList = folderMap.get(folderName);
if (uidList == null) {
uidList = new ArrayList<String>();
folderMap.put(folderName, uidList);
}
uidList.add(uid);
}
} finally {
cursor.close();
}
}
@Override
public void postDbWork() {
notifyChange();
}
}, UID_CHECK_BATCH_SIZE);
return folderMap;
}
}