From 9627d014e16235eadf981b9165807dc72a14a383 Mon Sep 17 00:00:00 2001 From: Mihai Preda Date: Wed, 12 Aug 2009 12:51:26 +0200 Subject: [PATCH] MessageCompose: update body on save. - plus unit tests. - and some attachment refactoring. - move PROJECTION_ID up to EmailContent. - add index on messageKey to Attachments and Body tables. - add missing Columns.ID field in EmailContent. --- src/com/android/email/Controller.java | 1 - .../email/activity/MessageCompose.java | 95 ++++++++----------- .../android/email/provider/EmailContent.java | 62 +++++++++--- .../android/email/provider/EmailProvider.java | 36 ++++--- .../exchange/adapter/FolderSyncParser.java | 3 +- .../android/email/provider/ProviderTests.java | 87 +++++++++++++++++ 6 files changed, 202 insertions(+), 82 deletions(-) diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index c14960473..878d217a4 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -286,7 +286,6 @@ public class Controller { * Send a message: * - move the message to Outbox (the message is assumed to be in Drafts). * - perform any necessary notification - * - do the work in a separate (non-UI) thread * @param messageId the id of the message to send */ public void sendMessage(long messageId, long accountId) { diff --git a/src/com/android/email/activity/MessageCompose.java b/src/com/android/email/activity/MessageCompose.java index f582c40c3..98bf16276 100644 --- a/src/com/android/email/activity/MessageCompose.java +++ b/src/com/android/email/activity/MessageCompose.java @@ -30,10 +30,10 @@ import com.android.email.mail.Part; import com.android.email.mail.Message.RecipientType; import com.android.email.mail.internet.EmailHtmlUtil; import com.android.email.mail.internet.MimeUtility; -import com.android.email.mail.store.LocalStore.LocalAttachmentBody; import com.android.email.provider.EmailContent; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.Body; +import com.android.email.provider.EmailContent.BodyColumns; import com.android.email.provider.EmailContent.Message; import com.android.email.provider.EmailContent.MessageColumns; @@ -171,7 +171,7 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus /** * Encapsulates known information about a single attachment. */ - private static class Attachment implements Serializable { + private static class AttachmentInfo { public String name; public String contentType; public long size; @@ -334,7 +334,7 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus ArrayList attachments = new ArrayList(); for (int i = 0, count = mAttachments.getChildCount(); i < count; i++) { View view = mAttachments.getChildAt(i); - Attachment attachment = (Attachment) view.getTag(); + AttachmentInfo attachment = (AttachmentInfo) view.getTag(); attachments.add(attachment.uri); } outState.putParcelableArrayList(STATE_KEY_ATTACHMENTS, attachments); @@ -708,7 +708,6 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus if (mDraft == null) { mDraft = new Message(); } - updateMessage(mDraft, mAccount, buildBodyText(mSource)); new AsyncTask() { @@ -716,28 +715,28 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus protected Void doInBackground(Void... params) { final String action = getIntent().getAction(); if (mDraft.isSaved()) { - mDraft.update(getApplication(), getUpdateContentValues(mDraft)); + mDraft.update(MessageCompose.this, getUpdateContentValues(mDraft)); + ContentValues values = new ContentValues(); + values.put(BodyColumns.TEXT_CONTENT, mDraft.mText); + Body.updateBodyWithMessageId(MessageCompose.this, mDraft.mId, values); } else { // saveToMailbox() is synchronous (in this same thread) // so that mDraft.mId is set upon return // (needed by the sendMessage() that happens afterwards). mController.saveToMailbox(mDraft, EmailContent.Mailbox.TYPE_DRAFTS); } + if (send) { + mController.sendMessage(mDraft.mId, mDraft.mAccountKey); + // After a send it's no longer a draft; null it here just to be sure, + // although MessageCompose should just finish() anyway. + mDraft = null; + } return null; } @Override protected void onPostExecute(Void dummy) { - if (send) { - mController.sendMessage(mDraft.mId, mDraft.mAccountKey); - // Decide whether we need to show a Toast with something like - // "Message saved to Outbox for sending" - // or whether just the dismissing of the composing activity is enough feedback - - // After a send it's no longer a draft; null it here just to be sure, - // although MessageCompose should just finish() anyway. - mDraft = null; - } else { + if (!send) { // Don't display the toast if the user is just changing the orientation if ((getChangingConfigurations() & ActivityInfo.CONFIG_ORIENTATION) == 0) { Toast.makeText(MessageCompose.this, getString(R.string.message_saved_toast), @@ -818,63 +817,51 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus ACTIVITY_REQUEST_PICK_ATTACHMENT); } - private void addAttachment(Uri uri) { - addAttachment(uri, -1, null); - } - - private void addAttachment(Uri uri, int size, String name) { + private AttachmentInfo loadAttachmentInfo(Uri uri) { + int size = -1; + String name = null; ContentResolver contentResolver = getContentResolver(); + Cursor metadataCursor = contentResolver.query(uri, + new String[]{ OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE }, + null, null, null); + if (metadataCursor != null) { + try { + if (metadataCursor.moveToFirst()) { + name = metadataCursor.getString(0); + size = metadataCursor.getInt(1); + } + } finally { + metadataCursor.close(); + } + } + if (name == null) { + name = uri.getLastPathSegment(); + } String contentType = contentResolver.getType(uri); - if (contentType == null) { contentType = ""; } - Attachment attachment = new Attachment(); + AttachmentInfo attachment = new AttachmentInfo(); attachment.name = name; attachment.contentType = contentType; attachment.size = size; attachment.uri = uri; + return attachment; + } - if (attachment.size == -1 || attachment.name == null) { - Cursor metadataCursor = contentResolver.query( - uri, - new String[]{ OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE }, - null, - null, - null); - if (metadataCursor != null) { - try { - if (metadataCursor.moveToFirst()) { - if (attachment.name == null) { - attachment.name = metadataCursor.getString(0); - } - if (attachment.size == -1) { - attachment.size = metadataCursor.getInt(1); - } - } - } finally { - metadataCursor.close(); - } - } - } - - if (attachment.name == null) { - attachment.name = uri.getLastPathSegment(); - } - + private void addAttachment(Uri uri) { + AttachmentInfo attachment = loadAttachmentInfo(uri); // Before attaching the attachment, make sure it meets any other pre-attach criteria if (attachment.size > Email.MAX_ATTACHMENT_UPLOAD_SIZE) { Toast.makeText(this, R.string.message_compose_attachment_size, Toast.LENGTH_LONG) .show(); return; } - - View view = getLayoutInflater().inflate( - R.layout.message_compose_attachment, - mAttachments, - false); + + View view = getLayoutInflater().inflate(R.layout.message_compose_attachment, + mAttachments, false); TextView nameView = (TextView)view.findViewById(R.id.attachment_name); ImageButton delete = (ImageButton)view.findViewById(R.id.attachment_delete); nameView.setText(attachment.name); diff --git a/src/com/android/email/provider/EmailContent.java b/src/com/android/email/provider/EmailContent.java index a51ac57a1..431d72da6 100644 --- a/src/com/android/email/provider/EmailContent.java +++ b/src/com/android/email/provider/EmailContent.java @@ -20,6 +20,7 @@ import com.android.email.R; import android.content.ContentProviderOperation; import android.content.ContentProviderResult; +import android.content.ContentResolver; import android.content.ContentUris; import android.content.ContentValues; import android.content.Context; @@ -60,8 +61,14 @@ import java.util.UUID; public abstract class EmailContent { public static final String AUTHORITY = EmailProvider.EMAIL_AUTHORITY; public static final Uri CONTENT_URI = Uri.parse("content://" + AUTHORITY); + // All classes share this + public static final String RECORD_ID = "_id"; private static final String[] COUNT_COLUMNS = new String[]{"count(*)"}; + public static final String[] ID_PROJECTION = new String[] { + RECORD_ID + }; + private static final int ID_PROJECTION_COLUMN = 0; // Newly created objects get this id private static final int NOT_SAVED = -1; @@ -152,10 +159,8 @@ public abstract class EmailContent { private EmailContent() { } - // All classes share this - public static final String RECORD_ID = "_id"; - public interface SyncColumns { + public static final String ID = "_id"; // source (account name and type) : foreign key into the AccountsProvider public static final String ACCOUNT_KEY = "syncAccountKey"; // source id (string) : the source's name of this item @@ -170,6 +175,7 @@ public abstract class EmailContent { } public interface BodyColumns { + public static final String ID = "_id"; // Foreign key to the message corresponding to this body public static final String MESSAGE_KEY = "messageKey"; // The html content itself @@ -249,6 +255,39 @@ public abstract class EmailContent { return restoreBodyWithCursor(c); } + /** + * Returns the bodyId for the given messageId, or -1 if no body is found. + */ + /* package */ + static long lookupBodyIdWithMessageId(ContentResolver resolver, long messageId) { + Cursor c = resolver.query(Body.CONTENT_URI, ID_PROJECTION, + Body.MESSAGE_KEY + "=?", + new String[] {Long.toString(messageId)}, null); + try { + return c.moveToFirst() ? c.getLong(ID_PROJECTION_COLUMN) : -1; + } finally { + c.close(); + } + } + + /** + * Updates the Body for a messageId with the given ContentValues. + * If the message has no body, a new body is inserted for the message. + * Warning: the argument "values" is modified by this method, setting MESSAGE_KEY. + */ + public static void updateBodyWithMessageId(Context context, long messageId, + ContentValues values) { + ContentResolver resolver = context.getContentResolver(); + long bodyId = lookupBodyIdWithMessageId(resolver, messageId); + values.put(BodyColumns.MESSAGE_KEY, messageId); + if (bodyId == -1) { + resolver.insert(CONTENT_URI, values); + } else { + final Uri uri = ContentUris.withAppendedId(CONTENT_URI, bodyId); + resolver.update(uri, values, null, null); + } + } + private static String restoreTextWithMessageId(Context context, long messageId, String[] projection) { Cursor c = context.getContentResolver().query(Body.CONTENT_URI, projection, @@ -289,6 +328,7 @@ public abstract class EmailContent { } public interface MessageColumns { + public static final String ID = "_id"; // Basic columns used in message list presentation // The name as shown to the user in a message list public static final String DISPLAY_NAME = "displayName"; @@ -722,6 +762,7 @@ public abstract class EmailContent { } public interface AccountColumns { + public static final String ID = "_id"; // The display name of the account (user-settable) public static final String DISPLAY_NAME = "displayName"; // The email address corresponding to this account @@ -813,13 +854,6 @@ public abstract class EmailContent { AccountColumns.RINGTONE_URI, AccountColumns.PROTOCOL_VERSION }; - /** - * This projection is for listing account id's only - */ - public static final String[] ID_PROJECTION = new String[] { - RECORD_ID - }; - public static final int CONTENT_MAILBOX_TYPE_COLUMN = 1; /** @@ -1462,6 +1496,7 @@ public abstract class EmailContent { } public interface AttachmentColumns { + public static final String ID = "_id"; // The display name of the attachment public static final String FILENAME = "fileName"; // The mime type of the attachment @@ -1739,9 +1774,6 @@ public abstract class EmailContent { private static final String WHERE_TYPE_AND_ACCOUNT_KEY = MailboxColumns.TYPE + "=? and " + MailboxColumns.ACCOUNT_KEY + "=?"; - private static final int ID_PROJECTION_ID = 0; - private static final String[] ID_PROJECTION = new String[] { ID }; - public Mailbox() { mBaseUri = CONTENT_URI; } @@ -1858,7 +1890,7 @@ public abstract class EmailContent { ID_PROJECTION, WHERE_TYPE_AND_ACCOUNT_KEY, bindArguments, null); try { if (c.moveToFirst()) { - mailboxId = c.getLong(ID_PROJECTION_ID); + mailboxId = c.getLong(ID_PROJECTION_COLUMN); } } finally { c.close(); @@ -2153,4 +2185,4 @@ public abstract class EmailContent { return getStoreUri(); } } -} \ No newline at end of file +} diff --git a/src/com/android/email/provider/EmailProvider.java b/src/com/android/email/provider/EmailProvider.java index 22805cb97..ebdc1edad 100644 --- a/src/com/android/email/provider/EmailProvider.java +++ b/src/com/android/email/provider/EmailProvider.java @@ -234,6 +234,18 @@ public class EmailProvider extends ContentProvider { matcher.addURI(EMAIL_AUTHORITY, "updatedMessage/#", UPDATED_MESSAGE_ID); } + /* + * Internal helper method for index creation. + * Example: + * "create index message_" + MessageColumns.FLAG_READ + * + " on " + Message.TABLE_NAME + " (" + MessageColumns.FLAG_READ + ");" + */ + /* package */ + static String createIndex(String tableName, String columnName) { + return "create index " + tableName.toLowerCase() + '_' + columnName + + " on " + tableName + " (" + columnName + ");"; + } + static void createMessageTable(SQLiteDatabase db) { String messageColumns = MessageColumns.DISPLAY_NAME + " text, " + MessageColumns.TIMESTAMP + " integer, " @@ -286,17 +298,17 @@ public class EmailProvider extends ContentProvider { db.execSQL("create table " + Message.UPDATED_TABLE_NAME + altCreateString); db.execSQL("create table " + Message.DELETED_TABLE_NAME + altCreateString); - // For now, indices only on the Message table - db.execSQL("create index message_" + MessageColumns.TIMESTAMP - + " on " + Message.TABLE_NAME + " (" + MessageColumns.TIMESTAMP + ");"); - db.execSQL("create index message_" + MessageColumns.FLAG_READ - + " on " + Message.TABLE_NAME + " (" + MessageColumns.FLAG_READ + ");"); - db.execSQL("create index message_" + MessageColumns.FLAG_LOADED - + " on " + Message.TABLE_NAME + " (" + MessageColumns.FLAG_LOADED + ");"); - db.execSQL("create index message_" + MessageColumns.MAILBOX_KEY - + " on " + Message.TABLE_NAME + " (" + MessageColumns.MAILBOX_KEY + ");"); - db.execSQL("create index message_" + SyncColumns.SERVER_ID - + " on " + Message.TABLE_NAME + " (" + SyncColumns.SERVER_ID + ");"); + String indexColumns[] = { + MessageColumns.TIMESTAMP, + MessageColumns.FLAG_READ, + MessageColumns.FLAG_LOADED, + MessageColumns.MAILBOX_KEY, + SyncColumns.SERVER_ID + }; + + for (String columnName : indexColumns) { + db.execSQL(createIndex(Message.TABLE_NAME, columnName)); + } // Deleting a Message deletes all associated Attachments // Deleting the associated Body cannot be done in a trigger, because the Body is stored @@ -465,6 +477,7 @@ public class EmailProvider extends ContentProvider { + AttachmentColumns.ENCODING + " text" + ");"; db.execSQL("create table " + Attachment.TABLE_NAME + s); + db.execSQL(createIndex(Attachment.TABLE_NAME, AttachmentColumns.MESSAGE_KEY)); } static void upgradeAttachmentTable(SQLiteDatabase db, int oldVersion, int newVersion) { @@ -482,6 +495,7 @@ public class EmailProvider extends ContentProvider { + BodyColumns.TEXT_CONTENT + " text" + ");"; db.execSQL("create table " + Body.TABLE_NAME + s); + db.execSQL(createIndex(Body.TABLE_NAME, BodyColumns.MESSAGE_KEY)); } static void upgradeBodyTable(SQLiteDatabase db, int oldVersion, int newVersion) { diff --git a/src/com/android/exchange/adapter/FolderSyncParser.java b/src/com/android/exchange/adapter/FolderSyncParser.java index 2b14e57bc..ac373030a 100644 --- a/src/com/android/exchange/adapter/FolderSyncParser.java +++ b/src/com/android/exchange/adapter/FolderSyncParser.java @@ -18,6 +18,7 @@ package com.android.exchange.adapter; import com.android.email.provider.EmailProvider; +import com.android.email.provider.EmailContent; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.AccountColumns; import com.android.email.provider.EmailContent.Mailbox; @@ -146,7 +147,7 @@ public class FolderSyncParser extends AbstractSyncParser { private Cursor getServerIdCursor(String serverId) { mBindArguments[0] = serverId; mBindArguments[1] = mAccountIdAsString; - return mContentResolver.query(Mailbox.CONTENT_URI, new String[] {MailboxColumns.ID}, + return mContentResolver.query(Mailbox.CONTENT_URI, EmailContent.ID_PROJECTION, WHERE_SERVER_ID_AND_ACCOUNT, mBindArguments, null); } diff --git a/tests/src/com/android/email/provider/ProviderTests.java b/tests/src/com/android/email/provider/ProviderTests.java index 1c2ad23a0..290018f46 100644 --- a/tests/src/com/android/email/provider/ProviderTests.java +++ b/tests/src/com/android/email/provider/ProviderTests.java @@ -20,6 +20,7 @@ import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.AccountColumns; import com.android.email.provider.EmailContent.Attachment; import com.android.email.provider.EmailContent.Body; +import com.android.email.provider.EmailContent.BodyColumns; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.MailboxColumns; import com.android.email.provider.EmailContent.Message; @@ -308,6 +309,81 @@ public class ProviderTests extends ProviderTestCase2 { assertEquals(0, numBoxes); } + /** + * Test for Body.lookupBodyIdWithMessageId() + * Verifies that: + * - for a message without body, -1 is returned. + * - for a mesage with body, the id matches the one from loadBodyForMessageId. + */ + public void testLookupBodyIdWithMessageId() { + final ContentResolver resolver = mMockContext.getContentResolver(); + Account account1 = ProviderTestUtils.setupAccount("orphaned body", true, mMockContext); + long account1Id = account1.mId; + Mailbox box1 = ProviderTestUtils.setupMailbox("box1", account1Id, true, mMockContext); + long box1Id = box1.mId; + + // 1. create message with no body, check that returned bodyId is -1 + Message message1 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, false, + true, mMockContext); + long message1Id = message1.mId; + long bodyId1 = Body.lookupBodyIdWithMessageId(resolver, message1Id); + assertEquals(bodyId1, -1); + + // 2. create message with body, check that returned bodyId is correct + Message message2 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, true, + true, mMockContext); + long message2Id = message2.mId; + long bodyId2 = Body.lookupBodyIdWithMessageId(resolver, message2Id); + Body body = loadBodyForMessageId(message2Id); + assertNotNull(body); + assertEquals(body.mId, bodyId2); + } + + /** + * Test for Body.updateBodyWithMessageId(). + * 1. - create message without body, + * - update its body (set TEXT_CONTENT) + * - check correct updated body is read back + * + * 2. - create message with body, + * - update body (set TEXT_CONTENT) + * - check correct updated body is read back + */ + public void testUpdateBodyWithMessageId() { + Account account1 = ProviderTestUtils.setupAccount("orphaned body", true, mMockContext); + long account1Id = account1.mId; + Mailbox box1 = ProviderTestUtils.setupMailbox("box1", account1Id, true, mMockContext); + long box1Id = box1.mId; + + final String textContent = "foobar some odd text"; + + ContentValues values = new ContentValues(); + values.put(BodyColumns.TEXT_CONTENT, textContent); + + // 1 + Message message1 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, false, + true, mMockContext); + long message1Id = message1.mId; + Body body1 = loadBodyForMessageId(message1Id); + assertNull(body1); + Body.updateBodyWithMessageId(mMockContext, message1Id, values); + body1 = loadBodyForMessageId(message1Id); + assertNotNull(body1); + assertEquals(body1.mTextContent, textContent); + + // 2 + Message message2 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, true, + true, mMockContext); + long message2Id = message2.mId; + Body body2 = loadBodyForMessageId(message2Id); + assertNotNull(body2); + assertTrue(!body2.mTextContent.equals(textContent)); + Body.updateBodyWithMessageId(mMockContext, message2Id, values); + body2 = loadBodyForMessageId(message1Id); + assertNotNull(body2); + assertEquals(body2.mTextContent, textContent); + } + /** * Test delete body. * 1. create message without body (message id 1) @@ -913,4 +989,15 @@ public class ProviderTests extends ProviderTestCase2 { assertEquals(1, getUnreadCount(boxB.mId)); assertEquals(2, getUnreadCount(boxC.mId)); } + + /** + * Test for EmailProvider.createIndex(). + * Check that it returns exacly the same string as the one used previously for index creation. + */ + public void testCreateIndex() { + String oldStr = "create index message_" + MessageColumns.TIMESTAMP + + " on " + Message.TABLE_NAME + " (" + MessageColumns.TIMESTAMP + ");"; + String newStr = EmailProvider.createIndex(Message.TABLE_NAME, MessageColumns.TIMESTAMP); + assertEquals(newStr, oldStr); + } }