Add an additional mailbox key column to message table

b/11294681
The problem is that when we try to open an attachment for a
message in search results, it fails. The reason is that part of
loading the attachment, we need to open the remote folder the
message is in. For search results, the message's mailboxKey is
the special fake "search_results" folder, which doesn't actually
exist on the server.
For this change, I've added a new column called "mainMailboxKey".
For search results, this column will be populated with the real
mailbox the message is in. It will be blank for other messages.

This is a quick and low risk fix for this bug, but it's kind
of awkward. We would prefer to do one or both of the following
some time after MR1.

1. Make the "search_results" folder be a virtual folder, the same
way that unread, starred, and other virtual folders are. For these,
there is actually no mailbox row in the database, just some
queries that check various flags in the messages and behave
like folders in the UI. The messages actually still reside in the
real folders.
2. Remove the requirement to open the folder at all to load the
attachment.

Change-Id: I825ab846f78bf8b041a5d1d579260dc5d7b4c522
This commit is contained in:
Martin Hibdon 2013-10-23 11:18:54 -07:00
parent bf39d1166c
commit c86fbb5bcb
4 changed files with 49 additions and 4 deletions

View File

@ -554,6 +554,10 @@ public abstract class EmailContent {
// References to other Email objects in the database
// Foreign key to the Mailbox holding this message [INDEX]
// TODO: This column is used in a complicated way: Usually, this refers to the mailbox
// the server considers this message to be in. In the case of search results, this key
// will refer to a special "search" mailbox, which does not exist on the server.
// This is confusing and causes problems, see b/11294681.
public static final String MAILBOX_KEY = "mailboxKey";
// Foreign key to the Account holding this message
public static final String ACCOUNT_KEY = "accountKey";
@ -580,6 +584,15 @@ public abstract class EmailContent {
/** Boolean, unseen = 0, seen = 1 [INDEX] */
public static final String FLAG_SEEN = "flagSeen";
// References to other Email objects in the database
// Foreign key to the Mailbox holding this message [INDEX]
// In cases where mailboxKey is NOT the real mailbox the server considers this message in,
// this will be set. See b/11294681
// We'd like to get rid of this column when the other changes mentioned in that bug
// can be addressed.
public static final String MAIN_MAILBOX_KEY = "mainMailboxKey";
}
public static final class Message extends EmailContent implements SyncColumns, MessageColumns {
@ -641,6 +654,7 @@ public abstract class EmailContent {
public static final int CONTENT_THREAD_TOPIC_COLUMN = 23;
public static final int CONTENT_SYNC_DATA_COLUMN = 24;
public static final int CONTENT_FLAG_SEEN_COLUMN = 25;
public static final int CONTENT_MAIN_MAILBOX_KEY_COLUMN = 26;
public static final String[] CONTENT_PROJECTION = new String[] {
RECORD_ID,
@ -655,7 +669,8 @@ public abstract class EmailContent {
MessageColumns.BCC_LIST, MessageColumns.REPLY_TO_LIST,
SyncColumns.SERVER_TIMESTAMP, MessageColumns.MEETING_INFO,
MessageColumns.SNIPPET, MessageColumns.PROTOCOL_SEARCH_INFO,
MessageColumns.THREAD_TOPIC, MessageColumns.SYNC_DATA, MessageColumns.FLAG_SEEN
MessageColumns.THREAD_TOPIC, MessageColumns.SYNC_DATA,
MessageColumns.FLAG_SEEN, MessageColumns.MAIN_MAILBOX_KEY
};
public static final int LIST_ID_COLUMN = 0;
@ -775,6 +790,7 @@ public abstract class EmailContent {
public long mMailboxKey;
public long mAccountKey;
public long mMainMailboxKey;
public String mFrom;
public String mTo;
@ -905,6 +921,7 @@ public abstract class EmailContent {
values.put(MessageColumns.PROTOCOL_SEARCH_INFO, mProtocolSearchInfo);
values.put(MessageColumns.THREAD_TOPIC, mThreadTopic);
values.put(MessageColumns.SYNC_DATA, mSyncData);
values.put(MessageColumns.MAIN_MAILBOX_KEY, mMainMailboxKey);
return values;
}
@ -931,6 +948,7 @@ public abstract class EmailContent {
mDraftInfo = cursor.getInt(CONTENT_DRAFT_INFO_COLUMN);
mMessageId = cursor.getString(CONTENT_MESSAGE_ID_COLUMN);
mMailboxKey = cursor.getLong(CONTENT_MAILBOX_KEY_COLUMN);
mMainMailboxKey = cursor.getLong(CONTENT_MAIN_MAILBOX_KEY_COLUMN);
mAccountKey = cursor.getLong(CONTENT_ACCOUNT_KEY_COLUMN);
mFrom = cursor.getString(CONTENT_FROM_LIST_COLUMN);
mTo = cursor.getString(CONTENT_TO_LIST_COLUMN);

View File

@ -156,7 +156,10 @@ public final class DBHelper {
// Version 118: Set syncInterval to 0 for all IMAP mailboxes
// Version 119: Disable syncing of DRAFTS type folders.
// Version 120: Changed duplicateMessage deletion trigger to ignore search mailboxes.
public static final int DATABASE_VERSION = 120;
// Version 121: Add mainMailboxKey, which will be set for messages that are in the fake
// "search_results" folder to reflect the mailbox that the server considers
// the message to be in. Also, wipe out any stale search_result folders.
public static final int DATABASE_VERSION = 121;
// Any changes to the database format *must* include update-in-place code.
// Original version: 2
@ -261,7 +264,8 @@ public final class DBHelper {
+ MessageColumns.PROTOCOL_SEARCH_INFO + " text, "
+ MessageColumns.THREAD_TOPIC + " text, "
+ MessageColumns.SYNC_DATA + " text, "
+ MessageColumns.FLAG_SEEN + " integer"
+ MessageColumns.FLAG_SEEN + " integer, "
+ MessageColumns.MAIN_MAILBOX_KEY + " integer"
+ ");";
// This String and the following String MUST have the same columns, except for the type
@ -1266,6 +1270,22 @@ public final class DBHelper {
createDeleteDuplicateMessagesTrigger(db);
}
// Add the mainMailboxKey column, and get rid of any messages in the search_results
// folder.
//
if (oldVersion <= 120) {
db.execSQL("alter table " + Message.TABLE_NAME
+ " add " + MessageColumns.MAIN_MAILBOX_KEY + " integer");
// Delete all TYPE_SEARCH mailboxes. These will be for stale queries anyway, and
// the messages in them will not have the mainMailboxKey column correctly populated.
// We have a trigger (See TRIGGER_MAILBOX_DELETE) that will delete any messages
// in the deleted mailboxes.
db.execSQL("delete from " + Mailbox.TABLE_NAME + " where "
+ Mailbox.TYPE + "=" + Mailbox.TYPE_SEARCH);
}
}
@Override

View File

@ -228,7 +228,7 @@ public abstract class EmailServiceStub extends IEmailService.Stub implements IEm
new String[] {BodyColumns.SOURCE_MESSAGE_KEY},
BodyColumns.MESSAGE_KEY + "=?",
new String[] {Long.toString(messageId)}, null, 0, -1L);
if (sourceId != -1 ) {
if (sourceId != -1) {
EmailContent.Message sourceMsg =
EmailContent.Message.restoreMessageWithId(mContext, sourceId);
if (sourceMsg != null) {
@ -236,6 +236,8 @@ public abstract class EmailServiceStub extends IEmailService.Stub implements IEm
message.mServerId = sourceMsg.mServerId;
}
}
} else if (mailbox.mType == Mailbox.TYPE_SEARCH && message.mMainMailboxKey != 0) {
mailbox = Mailbox.restoreMailboxWithId(mContext, message.mMainMailboxKey);
}
if (account == null || mailbox == null) {
@ -262,6 +264,7 @@ public abstract class EmailServiceStub extends IEmailService.Stub implements IEm
String.format("%s;\n name=\"%s\"",
attachment.mMimeType,
attachment.mFileName));
// TODO is this always true for attachments? I think we dropped the
// true encoding along the way
storePart.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64");

View File

@ -1477,6 +1477,10 @@ public class ImapService extends Service {
// Copy the fields that are available into the message
LegacyConversions.updateMessageFields(localMessage,
message, account.mId, mailbox.mId);
// Save off the mailbox that this message *really* belongs in.
// We need this information if we need to do more lookups
// (like loading attachments) for this message. See b/11294681
localMessage.mMainMailboxKey = localMessage.mMailboxKey;
localMessage.mMailboxKey = destMailboxId;
// We load 50k or so; maybe it's complete, maybe not...
int flag = EmailContent.Message.FLAG_LOADED_COMPLETE;