am 196f6187
: Merge change I036f39c6 into eclair-mr2
Merge commit '196f6187bb25c7ed1b2f6f5f3178152294b3cc22' into eclair-mr2-plus-aosp * commit '196f6187bb25c7ed1b2f6f5f3178152294b3cc22': Prevent duplication of POP3 attachments
This commit is contained in:
commit
3259aa9c74
@ -38,6 +38,7 @@ import org.apache.commons.io.IOUtils;
|
||||
import android.content.ContentUris;
|
||||
import android.content.ContentValues;
|
||||
import android.content.Context;
|
||||
import android.database.Cursor;
|
||||
import android.net.Uri;
|
||||
import android.util.Log;
|
||||
|
||||
@ -50,6 +51,9 @@ import java.util.Date;
|
||||
|
||||
public class LegacyConversions {
|
||||
|
||||
/** DO NOT CHECK IN "TRUE" */
|
||||
private static final boolean DEBUG_ATTACHMENTS = false;
|
||||
|
||||
/**
|
||||
* Values for HEADER_ANDROID_BODY_QUOTED_PART to tag body parts
|
||||
*/
|
||||
@ -255,8 +259,14 @@ public class LegacyConversions {
|
||||
/**
|
||||
* Add a single attachment part to the message
|
||||
*
|
||||
* TODO: This will simply add to any existing attachments - could this ever happen? If so,
|
||||
* change it to find existing attachments and delete/merge them.
|
||||
* This will skip adding attachments if they are already found in the attachments table.
|
||||
* The heuristic for this will fail (false-positive) if two identical attachments are
|
||||
* included in a single POP3 message.
|
||||
* TODO: Fix that, by (elsewhere) simulating an mLocation value based on the attachments
|
||||
* position within the list of multipart/mixed elements. This would make every POP3 attachment
|
||||
* unique, and might also simplify the code (since we could just look at the positions, and
|
||||
* ignore the filename, etc.)
|
||||
*
|
||||
* TODO: Take a closer look at encoding and deal with it if necessary.
|
||||
*
|
||||
* @param context a context for file operations
|
||||
@ -301,8 +311,44 @@ public class LegacyConversions {
|
||||
localAttachment.mLocation = partId;
|
||||
localAttachment.mEncoding = "B"; // TODO - convert other known encodings
|
||||
|
||||
if (DEBUG_ATTACHMENTS) {
|
||||
Log.d(Email.LOG_TAG, "Add attachment " + localAttachment);
|
||||
}
|
||||
|
||||
// To prevent duplication - do we already have a matching attachment?
|
||||
// The fields we'll check for equality are:
|
||||
// mFileName, mMimeType, mContentId, mMessageKey, mLocation
|
||||
// NOTE: This will false-positive if you attach the exact same file, twice, to a POP3
|
||||
// message. We can live with that - you'll get one of the copies.
|
||||
Uri uri = ContentUris.withAppendedId(Attachment.MESSAGE_ID_URI, localMessage.mId);
|
||||
Cursor cursor = context.getContentResolver().query(uri, Attachment.CONTENT_PROJECTION,
|
||||
null, null, null);
|
||||
boolean attachmentFoundInDb = false;
|
||||
try {
|
||||
while (cursor.moveToNext()) {
|
||||
Attachment dbAttachment = new Attachment().restore(cursor);
|
||||
// We test each of the fields here (instead of in SQL) because they may be
|
||||
// null, or may be strings.
|
||||
if (stringNotEqual(dbAttachment.mFileName, localAttachment.mFileName)) continue;
|
||||
if (stringNotEqual(dbAttachment.mMimeType, localAttachment.mMimeType)) continue;
|
||||
if (stringNotEqual(dbAttachment.mContentId, localAttachment.mContentId)) continue;
|
||||
if (stringNotEqual(dbAttachment.mLocation, localAttachment.mLocation)) continue;
|
||||
// We found a match, so use the existing attachment id, and stop looking/looping
|
||||
attachmentFoundInDb = true;
|
||||
localAttachment.mId = dbAttachment.mId;
|
||||
if (DEBUG_ATTACHMENTS) {
|
||||
Log.d(Email.LOG_TAG, "Skipped, found db attachment " + dbAttachment);
|
||||
}
|
||||
break;
|
||||
}
|
||||
} finally {
|
||||
cursor.close();
|
||||
}
|
||||
|
||||
// Save the attachment (so far) in order to obtain an id
|
||||
localAttachment.save(context);
|
||||
if (!attachmentFoundInDb) {
|
||||
localAttachment.save(context);
|
||||
}
|
||||
|
||||
// If an attachment body was actually provided, we need to write the file now
|
||||
saveAttachmentBody(context, part, localAttachment, localMessage.mAccountKey);
|
||||
@ -314,6 +360,17 @@ public class LegacyConversions {
|
||||
localMessage.mFlagAttachment = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper for addOneAttachment that compares two strings, deals with nulls, and treats
|
||||
* nulls and empty strings as equal.
|
||||
*/
|
||||
/* package */ static boolean stringNotEqual(String a, String b) {
|
||||
if (a == null && b == null) return false; // fast exit for two null strings
|
||||
if (a == null) a = "";
|
||||
if (b == null) b = "";
|
||||
return !a.equals(b);
|
||||
}
|
||||
|
||||
/**
|
||||
* Save the body part of a single attachment, to a file in the attachments directory.
|
||||
*/
|
||||
|
@ -1657,6 +1657,12 @@ public abstract class EmailContent {
|
||||
return new EmailContent.Attachment[size];
|
||||
}
|
||||
};
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "[" + mFileName + ", " + mMimeType + ", " + mSize + ", " + mContentId + ", "
|
||||
+ mContentUri + ", " + mMessageKey + ", " + mLocation + ", " + mEncoding + "]";
|
||||
}
|
||||
}
|
||||
|
||||
public interface MailboxColumns {
|
||||
|
@ -203,31 +203,7 @@ public class LegacyConversionsTests extends ProviderTestCase2<EmailProvider> {
|
||||
"local-message", accountId, mailboxId, false, true, mProviderContext);
|
||||
|
||||
// Prepare a legacy message with attachments
|
||||
Part attachment1Part = MessageTestUtils.bodyPart("image/gif", null);
|
||||
attachment1Part.setHeader(MimeHeader.HEADER_CONTENT_TYPE,
|
||||
"image/gif;\n name=\"attachment1\"");
|
||||
attachment1Part.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64");
|
||||
attachment1Part.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION,
|
||||
"attachment;\n filename=\"attachment1\";\n size=100");
|
||||
attachment1Part.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "101");
|
||||
|
||||
Part attachment2Part = MessageTestUtils.bodyPart("image/jpg", null);
|
||||
attachment2Part.setHeader(MimeHeader.HEADER_CONTENT_TYPE,
|
||||
"image/jpg;\n name=\"attachment2\"");
|
||||
attachment2Part.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64");
|
||||
attachment2Part.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION,
|
||||
"attachment;\n filename=\"attachment2\";\n size=200");
|
||||
attachment2Part.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "102");
|
||||
|
||||
final Message legacyMessage = new MessageBuilder()
|
||||
.setBody(new MultipartBuilder("multipart/mixed")
|
||||
.addBodyPart(MessageTestUtils.bodyPart("text/html", null))
|
||||
.addBodyPart(new MultipartBuilder("multipart/mixed")
|
||||
.addBodyPart((BodyPart)attachment1Part)
|
||||
.addBodyPart((BodyPart)attachment2Part)
|
||||
.buildBodyPart())
|
||||
.build())
|
||||
.build();
|
||||
final Message legacyMessage = prepareLegacyMessageWithAttachments(2);
|
||||
|
||||
// Now, convert from legacy to provider and see what happens
|
||||
ArrayList<Part> viewables = new ArrayList<Part>();
|
||||
@ -244,9 +220,9 @@ public class LegacyConversionsTests extends ProviderTestCase2<EmailProvider> {
|
||||
while (c.moveToNext()) {
|
||||
Attachment attachment = Attachment.getContent(c, Attachment.class);
|
||||
if ("101".equals(attachment.mLocation)) {
|
||||
checkAttachment("attachment1Part", attachment1Part, attachment);
|
||||
checkAttachment("attachment1Part", attachments.get(0), attachment);
|
||||
} else if ("102".equals(attachment.mLocation)) {
|
||||
checkAttachment("attachment2Part", attachment2Part, attachment);
|
||||
checkAttachment("attachment2Part", attachments.get(1), attachment);
|
||||
} else {
|
||||
fail("Unexpected attachment with location " + attachment.mLocation);
|
||||
}
|
||||
@ -256,6 +232,93 @@ public class LegacyConversionsTests extends ProviderTestCase2<EmailProvider> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that attachments aren't re-added in the DB. This supports the "partial download"
|
||||
* nature of POP messages.
|
||||
*/
|
||||
public void testAddDuplicateAttachments() throws MessagingException, IOException {
|
||||
// Prepare a local message to add the attachments to
|
||||
final long accountId = 1;
|
||||
final long mailboxId = 1;
|
||||
final EmailContent.Message localMessage = ProviderTestUtils.setupMessage(
|
||||
"local-message", accountId, mailboxId, false, true, mProviderContext);
|
||||
|
||||
// Prepare a legacy message with attachments
|
||||
Message legacyMessage = prepareLegacyMessageWithAttachments(2);
|
||||
|
||||
// Now, convert from legacy to provider and see what happens
|
||||
ArrayList<Part> viewables = new ArrayList<Part>();
|
||||
ArrayList<Part> attachments = new ArrayList<Part>();
|
||||
MimeUtility.collectParts(legacyMessage, viewables, attachments);
|
||||
LegacyConversions.updateAttachments(mProviderContext, localMessage, attachments);
|
||||
|
||||
// Confirm two attachment objects created
|
||||
Uri uri = ContentUris.withAppendedId(Attachment.MESSAGE_ID_URI, localMessage.mId);
|
||||
assertEquals(2, EmailContent.count(mProviderContext, uri, null, null));
|
||||
|
||||
// Now add the attachments again and confirm there are still only two
|
||||
LegacyConversions.updateAttachments(mProviderContext, localMessage, attachments);
|
||||
assertEquals(2, EmailContent.count(mProviderContext, uri, null, null));
|
||||
|
||||
// Now add a 3rd & 4th attachment and make sure the total is 4, not 2 or 6
|
||||
legacyMessage = prepareLegacyMessageWithAttachments(4);
|
||||
viewables = new ArrayList<Part>();
|
||||
attachments = new ArrayList<Part>();
|
||||
MimeUtility.collectParts(legacyMessage, viewables, attachments);
|
||||
LegacyConversions.updateAttachments(mProviderContext, localMessage, attachments);
|
||||
assertEquals(4, EmailContent.count(mProviderContext, uri, null, null));
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare a legacy message with 1+ attachments
|
||||
*/
|
||||
private static Message prepareLegacyMessageWithAttachments(int numAttachments)
|
||||
throws MessagingException {
|
||||
// First, build one or more attachment parts
|
||||
MultipartBuilder mpBuilder = new MultipartBuilder("multipart/mixed");
|
||||
for (int i = 1; i <= numAttachments; ++i) {
|
||||
BodyPart attachmentPart = MessageTestUtils.bodyPart("image/jpg", null);
|
||||
|
||||
// name=attachmentN size=N00 location=10N
|
||||
attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TYPE,
|
||||
"image/jpg;\n name=\"attachment" + i + "\"");
|
||||
attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_TRANSFER_ENCODING, "base64");
|
||||
attachmentPart.setHeader(MimeHeader.HEADER_CONTENT_DISPOSITION,
|
||||
"attachment;\n filename=\"attachment2\";\n size=" + i + "00");
|
||||
attachmentPart.setHeader(MimeHeader.HEADER_ANDROID_ATTACHMENT_STORE_DATA, "10" + i);
|
||||
|
||||
mpBuilder.addBodyPart(attachmentPart);
|
||||
}
|
||||
|
||||
// Now build a message with them
|
||||
final Message legacyMessage = new MessageBuilder()
|
||||
.setBody(new MultipartBuilder("multipart/mixed")
|
||||
.addBodyPart(MessageTestUtils.bodyPart("text/html", null))
|
||||
.addBodyPart(mpBuilder.buildBodyPart())
|
||||
.build())
|
||||
.build();
|
||||
|
||||
return legacyMessage;
|
||||
}
|
||||
|
||||
/**
|
||||
* Test the stringInequal helper
|
||||
*/
|
||||
public void testStringInequal() {
|
||||
// Pairs that are "equal"
|
||||
assertFalse(LegacyConversions.stringNotEqual(null, null));
|
||||
assertFalse(LegacyConversions.stringNotEqual(null, ""));
|
||||
assertFalse(LegacyConversions.stringNotEqual("", null));
|
||||
assertFalse(LegacyConversions.stringNotEqual("", ""));
|
||||
assertFalse(LegacyConversions.stringNotEqual("string-equal", "string-equal"));
|
||||
// Pairs that are "inequal"
|
||||
assertTrue(LegacyConversions.stringNotEqual(null, "string-inequal"));
|
||||
assertTrue(LegacyConversions.stringNotEqual("", "string-inequal"));
|
||||
assertTrue(LegacyConversions.stringNotEqual("string-inequal", null));
|
||||
assertTrue(LegacyConversions.stringNotEqual("string-inequal", ""));
|
||||
assertTrue(LegacyConversions.stringNotEqual("string-inequal-a", "string-inequal-b"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Compare attachment that was converted from Part (expected) to Provider Attachment (actual)
|
||||
*
|
||||
@ -345,7 +408,7 @@ public class LegacyConversionsTests extends ProviderTestCase2<EmailProvider> {
|
||||
assertEquals(tag, expect.isSet(Flag.FLAGGED), actual.mFlagFavorite);
|
||||
}
|
||||
|
||||
/**
|
||||
/**
|
||||
* Check equality of a pair of converted messages
|
||||
*/
|
||||
private void checkLegacyMessage(String tag, EmailContent.Message expect, Message actual)
|
||||
|
Loading…
Reference in New Issue
Block a user