ADHOC/replicant-packages_apps_Email
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

DO NOT MERGE. Prevent duplication of POP3 attachments

Browse Source 
* For each attachment we add, check the DB for an existing attachment
  with similar metadata (name, mime type, content id, etc.)
* Skip adding them if already held
* Unit tests

Originally fixed in 5b0a12c199 / CL I036f39c6

Fixes bug http://b/2084704
This commit is contained in:
Andrew Stadler 2009-10-14 12:41:05 -07:00
parent dee4e25320
commit 53123c2f91
3 changed files with 157 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
src/com/android/email/LegacyConversions.java
View File

@ -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.
*/

6
src/com/android/email/provider/EmailContent.java
View File

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

119
tests/src/com/android/email/LegacyConversionsTests.java
View File

@ -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)