Fix image cache bugs.

1) Index SlideDeckListener cache by MMS (id, timestamp) tuple.

2) Index parts by (id, content_id) tuples.

Fixes #840
Closes #3183
// FREEBIE
This commit is contained in:
Moxie Marlinspike 2015-05-18 08:16:06 -07:00
parent 082985276f
commit dc903e49af
8 changed files with 113 additions and 45 deletions

View file

@ -165,8 +165,8 @@ public class MmsDatabase extends MessagingDatabase {
}; };
public static final ExecutorService slideResolver = org.thoughtcrime.securesms.util.Util.newSingleThreadedLifoExecutor(); public static final ExecutorService slideResolver = org.thoughtcrime.securesms.util.Util.newSingleThreadedLifoExecutor();
private static final Map<Long, SoftReference<SlideDeck>> slideCache = private static final Map<String, SoftReference<SlideDeck>> slideCache =
Collections.synchronizedMap(new LRUCache<Long, SoftReference<SlideDeck>>(20)); Collections.synchronizedMap(new LRUCache<String, SoftReference<SlideDeck>>(20));
private final JobManager jobManager; private final JobManager jobManager;
@ -1045,7 +1045,7 @@ public class MmsDatabase extends MessagingDatabase {
List<IdentityKeyMismatch> mismatches = getMismatchedIdentities(mismatchDocument); List<IdentityKeyMismatch> mismatches = getMismatchedIdentities(mismatchDocument);
List<NetworkFailure> networkFailures = getFailures(networkDocument); List<NetworkFailure> networkFailures = getFailures(networkDocument);
ListenableFutureTask<SlideDeck> slideDeck = getSlideDeck(masterSecret, id); ListenableFutureTask<SlideDeck> slideDeck = getSlideDeck(masterSecret, dateReceived, id);
return new MediaMmsMessageRecord(context, id, recipients, recipients.getPrimaryRecipient(), return new MediaMmsMessageRecord(context, id, recipients, recipients.getPrimaryRecipient(),
addressDeviceId, dateSent, dateReceived, receiptCount, addressDeviceId, dateSent, dateReceived, receiptCount,
@ -1109,9 +1109,10 @@ public class MmsDatabase extends MessagingDatabase {
} }
private ListenableFutureTask<SlideDeck> getSlideDeck(final MasterSecret masterSecret, private ListenableFutureTask<SlideDeck> getSlideDeck(final MasterSecret masterSecret,
final long timestamp,
final long id) final long id)
{ {
ListenableFutureTask<SlideDeck> future = getCachedSlideDeck(id); ListenableFutureTask<SlideDeck> future = getCachedSlideDeck(timestamp, id);
if (future != null) { if (future != null) {
return future; return future;
@ -1128,21 +1129,21 @@ public class MmsDatabase extends MessagingDatabase {
SlideDeck slideDeck = new SlideDeck(context, masterSecret, body); SlideDeck slideDeck = new SlideDeck(context, masterSecret, body);
if (!body.containsPushInProgress()) { if (!body.containsPushInProgress()) {
slideCache.put(id, new SoftReference<SlideDeck>(slideDeck)); slideCache.put(timestamp + "::" + id, new SoftReference<>(slideDeck));
} }
return slideDeck; return slideDeck;
} }
}; };
future = new ListenableFutureTask<SlideDeck>(task); future = new ListenableFutureTask<>(task);
slideResolver.execute(future); slideResolver.execute(future);
return future; return future;
} }
private ListenableFutureTask<SlideDeck> getCachedSlideDeck(final long id) { private ListenableFutureTask<SlideDeck> getCachedSlideDeck(final long timestamp, final long id) {
SoftReference<SlideDeck> reference = slideCache.get(id); SoftReference<SlideDeck> reference = slideCache.get(timestamp + "::" + id);
if (reference != null) { if (reference != null) {
final SlideDeck slideDeck = reference.get(); final SlideDeck slideDeck = reference.get();
@ -1155,7 +1156,7 @@ public class MmsDatabase extends MessagingDatabase {
} }
}; };
ListenableFutureTask<SlideDeck> future = new ListenableFutureTask<SlideDeck>(task); ListenableFutureTask<SlideDeck> future = new ListenableFutureTask<>(task);
future.run(); future.run();
return future; return future;

View file

@ -79,6 +79,8 @@ public class PartDatabase extends Database {
private static final String THUMBNAIL = "thumbnail"; private static final String THUMBNAIL = "thumbnail";
private static final String ASPECT_RATIO = "aspect_ratio"; private static final String ASPECT_RATIO = "aspect_ratio";
private static final String ID_CONTENT_WHERE = ID + " = ? AND " + CONTENT_ID + " = ?";
public static final String CREATE_TABLE = "CREATE TABLE " + TABLE_NAME + " (" + ID + " INTEGER PRIMARY KEY, " + public static final String CREATE_TABLE = "CREATE TABLE " + TABLE_NAME + " (" + ID + " INTEGER PRIMARY KEY, " +
MMS_ID + " INTEGER, " + SEQUENCE + " INTEGER DEFAULT 0, " + MMS_ID + " INTEGER, " + SEQUENCE + " INTEGER DEFAULT 0, " +
CONTENT_TYPE + " TEXT, " + NAME + " TEXT, " + CHARSET + " INTEGER, " + CONTENT_TYPE + " TEXT, " + NAME + " TEXT, " + CHARSET + " INTEGER, " +
@ -113,10 +115,10 @@ public class PartDatabase extends Database {
super(context, databaseHelper); super(context, databaseHelper);
} }
public InputStream getPartStream(MasterSecret masterSecret, long partId) public InputStream getPartStream(MasterSecret masterSecret, long partId, byte[] contentId)
throws FileNotFoundException throws FileNotFoundException
{ {
return getDataStream(masterSecret, partId, DATA); return getDataStream(masterSecret, partId, contentId, DATA);
} }
public void updateFailedDownloadedPart(long messageId, long partId, PduPart part) public void updateFailedDownloadedPart(long messageId, long partId, PduPart part)
@ -343,15 +345,17 @@ public class PartDatabase extends Database {
return new EncryptingPartOutputStream(path, masterSecret); return new EncryptingPartOutputStream(path, masterSecret);
} }
@VisibleForTesting InputStream getDataStream(MasterSecret masterSecret, long partId, String dataType) @VisibleForTesting InputStream getDataStream(MasterSecret masterSecret, long partId, byte[] contentId, String dataType)
throws FileNotFoundException throws FileNotFoundException
{ {
SQLiteDatabase database = databaseHelper.getReadableDatabase(); SQLiteDatabase database = databaseHelper.getReadableDatabase();
Cursor cursor = null; Cursor cursor = null;
try { try {
cursor = database.query(TABLE_NAME, new String[]{dataType}, ID_WHERE, cursor = database.query(TABLE_NAME, new String[]{dataType}, ID_CONTENT_WHERE,
new String[] {partId+""}, null, null, null); new String[] {String.valueOf(partId),
Util.toIsoString(contentId)},
null, null, null);
if (cursor != null && cursor.moveToFirst()) { if (cursor != null && cursor.moveToFirst()) {
if (cursor.isNull(0)) { if (cursor.isNull(0)) {
@ -402,15 +406,15 @@ public class PartDatabase extends Database {
} }
} }
public InputStream getThumbnailStream(final MasterSecret masterSecret, final long partId) throws IOException { public InputStream getThumbnailStream(MasterSecret masterSecret, long partId, byte[] contentId) throws IOException {
Log.w(TAG, "getThumbnailStream(" + partId + ")"); Log.w(TAG, "getThumbnailStream(" + partId + ")");
final InputStream dataStream = getDataStream(masterSecret, partId, THUMBNAIL); final InputStream dataStream = getDataStream(masterSecret, partId, contentId, THUMBNAIL);
if (dataStream != null) { if (dataStream != null) {
return dataStream; return dataStream;
} }
try { try {
return thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)).get(); return thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, contentId)).get();
} catch (InterruptedException ie) { } catch (InterruptedException ie) {
throw new AssertionError("interrupted"); throw new AssertionError("interrupted");
} catch (ExecutionException ee) { } catch (ExecutionException ee) {
@ -424,7 +428,7 @@ public class PartDatabase extends Database {
getPartValues(part, cursor); getPartValues(part, cursor);
part.setDataUri(ContentUris.withAppendedId(PartAuthority.PART_CONTENT_URI, part.getId())); part.setDataUri(PartAuthority.getPartUri(part.getId(), part.getContentId()));
return part; return part;
} }
@ -454,7 +458,7 @@ public class PartDatabase extends Database {
ThumbnailData data = new ThumbnailData(thumbnail); ThumbnailData data = new ThumbnailData(thumbnail);
updatePartThumbnail(masterSecret, partId, part, data.toDataStream(), data.getAspectRatio()); updatePartThumbnail(masterSecret, partId, part, data.toDataStream(), data.getAspectRatio());
} else if (!part.isPendingPush()) { } else if (!part.isPendingPush()) {
thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)); thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, part.getContentId()));
} }
return partId; return partId;
@ -479,7 +483,7 @@ public class PartDatabase extends Database {
database.update(TABLE_NAME, values, ID_WHERE, new String[]{partId+""}); database.update(TABLE_NAME, values, ID_WHERE, new String[]{partId+""});
thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)); thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, part.getContentId()));
notifyConversationListeners(DatabaseFactory.getMmsDatabase(context).getThreadIdForMessage(messageId)); notifyConversationListeners(DatabaseFactory.getMmsDatabase(context).getThreadIdForMessage(messageId));
} }
@ -534,11 +538,12 @@ public class PartDatabase extends Database {
public static class ImageRecord { public static class ImageRecord {
private long partId; private long partId;
private byte[] contentId;
private String contentType; private String contentType;
private String address; private String address;
private long date; private long date;
private ImageRecord(long partId, String contentType, String address, long date) { private ImageRecord(long partId, byte[] contentId, String contentType, String address, long date) {
this.partId = partId; this.partId = partId;
this.contentType = contentType; this.contentType = contentType;
this.address = address; this.address = address;
@ -547,6 +552,7 @@ public class PartDatabase extends Database {
public static ImageRecord from(Cursor cursor) { public static ImageRecord from(Cursor cursor) {
return new ImageRecord(cursor.getLong(cursor.getColumnIndexOrThrow(ID)), return new ImageRecord(cursor.getLong(cursor.getColumnIndexOrThrow(ID)),
Util.toIsoBytes(cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_ID))),
cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_TYPE)), cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_TYPE)),
cursor.getString(cursor.getColumnIndexOrThrow(MmsDatabase.ADDRESS)), cursor.getString(cursor.getColumnIndexOrThrow(MmsDatabase.ADDRESS)),
cursor.getLong(cursor.getColumnIndexOrThrow(MmsDatabase.NORMALIZED_DATE_RECEIVED)) * 1000); cursor.getLong(cursor.getColumnIndexOrThrow(MmsDatabase.NORMALIZED_DATE_RECEIVED)) * 1000);
@ -569,22 +575,24 @@ public class PartDatabase extends Database {
} }
public Uri getUri() { public Uri getUri() {
return ContentUris.withAppendedId(PartAuthority.PART_CONTENT_URI, getPartId()); return PartAuthority.getPartUri(partId, contentId);
} }
} }
@VisibleForTesting class ThumbnailFetchCallable implements Callable<InputStream> { @VisibleForTesting class ThumbnailFetchCallable implements Callable<InputStream> {
private final MasterSecret masterSecret; private final MasterSecret masterSecret;
private final long partId; private final long partId;
private final byte[] contentId;
public ThumbnailFetchCallable(MasterSecret masterSecret, long partId) { public ThumbnailFetchCallable(MasterSecret masterSecret, long partId, byte[] contentId) {
this.masterSecret = masterSecret; this.masterSecret = masterSecret;
this.partId = partId; this.partId = partId;
this.contentId = contentId;
} }
@Override @Override
public InputStream call() throws Exception { public InputStream call() throws Exception {
final InputStream stream = getDataStream(masterSecret, partId, THUMBNAIL); final InputStream stream = getDataStream(masterSecret, partId, contentId, THUMBNAIL);
if (stream != null) { if (stream != null) {
return stream; return stream;
} }
@ -599,7 +607,8 @@ public class PartDatabase extends Database {
} catch (BitmapDecodingException bde) { } catch (BitmapDecodingException bde) {
throw new IOException(bde); throw new IOException(bde);
} }
return getDataStream(masterSecret, partId, THUMBNAIL);
return getDataStream(masterSecret, partId, contentId, THUMBNAIL);
} }
} }
} }

View file

@ -46,7 +46,7 @@ public class ImageSlide extends Slide {
if (!getPart().isPendingPush() && getPart().getDataUri() != null) { if (!getPart().isPendingPush() && getPart().getDataUri() != null) {
return isDraft() return isDraft()
? getPart().getDataUri() ? getPart().getDataUri()
: PartAuthority.getThumbnailUri(getPart().getId()); : PartAuthority.getThumbnailUri(getPart().getId(), part.getContentId());
} }
return null; return null;

View file

@ -32,6 +32,10 @@ public class IncomingMediaMessage {
this.body = retreived.getBody(); this.body = retreived.getBody();
this.groupId = null; this.groupId = null;
this.push = false; this.push = false;
for (int i=0;i<body.getPartsNum();i++) {
body.getPart(i).setContentId(String.valueOf(System.currentTimeMillis()).getBytes());
}
} }
public IncomingMediaMessage(MasterSecret masterSecret, public IncomingMediaMessage(MasterSecret masterSecret,

View file

@ -9,6 +9,7 @@ import org.thoughtcrime.securesms.crypto.MasterSecret;
import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.DatabaseFactory;
import org.thoughtcrime.securesms.database.PartDatabase; import org.thoughtcrime.securesms.database.PartDatabase;
import org.thoughtcrime.securesms.providers.PartProvider; import org.thoughtcrime.securesms.providers.PartProvider;
import org.thoughtcrime.securesms.util.Hex;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
@ -17,8 +18,8 @@ public class PartAuthority {
private static final String PART_URI_STRING = "content://org.thoughtcrime.securesms/part"; private static final String PART_URI_STRING = "content://org.thoughtcrime.securesms/part";
private static final String THUMB_URI_STRING = "content://org.thoughtcrime.securesms/thumb"; private static final String THUMB_URI_STRING = "content://org.thoughtcrime.securesms/thumb";
public static final Uri PART_CONTENT_URI = Uri.parse(PART_URI_STRING); private static final Uri PART_CONTENT_URI = Uri.parse(PART_URI_STRING);
public static final Uri THUMB_CONTENT_URI = Uri.parse(THUMB_URI_STRING); private static final Uri THUMB_CONTENT_URI = Uri.parse(THUMB_URI_STRING);
private static final int PART_ROW = 1; private static final int PART_ROW = 1;
private static final int THUMB_ROW = 2; private static final int THUMB_ROW = 2;
@ -27,8 +28,8 @@ public class PartAuthority {
static { static {
uriMatcher = new UriMatcher(UriMatcher.NO_MATCH); uriMatcher = new UriMatcher(UriMatcher.NO_MATCH);
uriMatcher.addURI("org.thoughtcrime.securesms", "part/#", PART_ROW); uriMatcher.addURI("org.thoughtcrime.securesms", "part/*/#", PART_ROW);
uriMatcher.addURI("org.thoughtcrime.securesms", "thumb/#", THUMB_ROW); uriMatcher.addURI("org.thoughtcrime.securesms", "thumb/*/#", THUMB_ROW);
} }
public static InputStream getPartStream(Context context, MasterSecret masterSecret, Uri uri) public static InputStream getPartStream(Context context, MasterSecret masterSecret, Uri uri)
@ -39,9 +40,14 @@ public class PartAuthority {
try { try {
switch (match) { switch (match) {
case PART_ROW: return partDatabase.getPartStream(masterSecret, ContentUris.parseId(uri)); case PART_ROW:
case THUMB_ROW: return partDatabase.getThumbnailStream(masterSecret, ContentUris.parseId(uri)); PartUri partUri = new PartUri(uri);
default: return context.getContentResolver().openInputStream(uri); return partDatabase.getPartStream(masterSecret, partUri.getId(), partUri.getContentId());
case THUMB_ROW:
partUri = new PartUri(uri);
return partDatabase.getThumbnailStream(masterSecret, partUri.getId(), partUri.getContentId());
default:
return context.getContentResolver().openInputStream(uri);
} }
} catch (SecurityException se) { } catch (SecurityException se) {
throw new IOException(se); throw new IOException(se);
@ -49,10 +55,17 @@ public class PartAuthority {
} }
public static Uri getPublicPartUri(Uri uri) { public static Uri getPublicPartUri(Uri uri) {
return ContentUris.withAppendedId(PartProvider.CONTENT_URI, ContentUris.parseId(uri)); PartUri partUri = new PartUri(uri);
return PartProvider.getContentUri(partUri.getId(), partUri.getContentId());
} }
public static Uri getThumbnailUri(long partId) { public static Uri getPartUri(long partId, byte[] contentId) {
return ContentUris.withAppendedId(THUMB_CONTENT_URI, partId); Uri uri = Uri.withAppendedPath(PART_CONTENT_URI, Hex.toStringCondensed(contentId));
return ContentUris.withAppendedId(uri, partId);
}
public static Uri getThumbnailUri(long partId, byte[] contentId) {
Uri uri = Uri.withAppendedPath(THUMB_CONTENT_URI, Hex.toStringCondensed(contentId));
return ContentUris.withAppendedId(uri, partId);
} }
} }

View file

@ -0,0 +1,30 @@
package org.thoughtcrime.securesms.mms;
import android.content.ContentUris;
import android.net.Uri;
import org.thoughtcrime.securesms.util.Hex;
import java.io.IOException;
public class PartUri {
private final Uri uri;
public PartUri(Uri uri) {
this.uri = uri;
}
public long getId() {
return ContentUris.parseId(uri);
}
public byte[] getContentId() {
try {
return Hex.fromStringCondensed(uri.getPathSegments().get(1));
} catch (IOException e) {
throw new AssertionError(e);
}
}
}

View file

@ -17,6 +17,7 @@
package org.thoughtcrime.securesms.providers; package org.thoughtcrime.securesms.providers;
import android.content.ContentProvider; import android.content.ContentProvider;
import android.content.ContentUris;
import android.content.ContentValues; import android.content.ContentValues;
import android.content.UriMatcher; import android.content.UriMatcher;
import android.database.Cursor; import android.database.Cursor;
@ -26,7 +27,9 @@ import android.util.Log;
import org.thoughtcrime.securesms.crypto.MasterSecret; import org.thoughtcrime.securesms.crypto.MasterSecret;
import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.DatabaseFactory;
import org.thoughtcrime.securesms.mms.PartUri;
import org.thoughtcrime.securesms.service.KeyCachingService; import org.thoughtcrime.securesms.service.KeyCachingService;
import org.thoughtcrime.securesms.util.Hex;
import java.io.File; import java.io.File;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
@ -38,14 +41,14 @@ public class PartProvider extends ContentProvider {
private static final String TAG = PartProvider.class.getSimpleName(); private static final String TAG = PartProvider.class.getSimpleName();
private static final String CONTENT_URI_STRING = "content://org.thoughtcrime.provider.securesms/part"; private static final String CONTENT_URI_STRING = "content://org.thoughtcrime.provider.securesms/part";
public static final Uri CONTENT_URI = Uri.parse(CONTENT_URI_STRING); private static final Uri CONTENT_URI = Uri.parse(CONTENT_URI_STRING);
private static final int SINGLE_ROW = 1; private static final int SINGLE_ROW = 1;
private static final UriMatcher uriMatcher; private static final UriMatcher uriMatcher;
static { static {
uriMatcher = new UriMatcher(UriMatcher.NO_MATCH); uriMatcher = new UriMatcher(UriMatcher.NO_MATCH);
uriMatcher.addURI("org.thoughtcrime.provider.securesms", "part/#", SINGLE_ROW); uriMatcher.addURI("org.thoughtcrime.provider.securesms", "part/*/#", SINGLE_ROW);
} }
@Override @Override
@ -54,8 +57,13 @@ public class PartProvider extends ContentProvider {
return true; return true;
} }
private File copyPartToTemporaryFile(MasterSecret masterSecret, long partId) throws IOException { public static Uri getContentUri(long partId, byte[] contentId) {
InputStream in = DatabaseFactory.getPartDatabase(getContext()).getPartStream(masterSecret, partId); Uri uri = Uri.withAppendedPath(CONTENT_URI, Hex.toStringCondensed(contentId));
return ContentUris.withAppendedId(uri, partId);
}
private File copyPartToTemporaryFile(MasterSecret masterSecret, long partId, byte[] contentId) throws IOException {
InputStream in = DatabaseFactory.getPartDatabase(getContext()).getPartStream(masterSecret, partId, contentId);
File tmpDir = getContext().getDir("tmp", 0); File tmpDir = getContext().getDir("tmp", 0);
File tmpFile = File.createTempFile("test", ".jpg", tmpDir); File tmpFile = File.createTempFile("test", ".jpg", tmpDir);
FileOutputStream fout = new FileOutputStream(tmpFile); FileOutputStream fout = new FileOutputStream(tmpFile);
@ -85,12 +93,14 @@ public class PartProvider extends ContentProvider {
case SINGLE_ROW: case SINGLE_ROW:
Log.w(TAG, "Parting out a single row..."); Log.w(TAG, "Parting out a single row...");
try { try {
int partId = Integer.parseInt(uri.getPathSegments().get(1)); PartUri partUri = new PartUri(uri);
File tmpFile = copyPartToTemporaryFile(masterSecret, partId); File tmpFile = copyPartToTemporaryFile(masterSecret, partUri.getId(), partUri.getContentId());
ParcelFileDescriptor pdf = ParcelFileDescriptor.open(tmpFile, ParcelFileDescriptor.MODE_READ_ONLY); ParcelFileDescriptor pdf = ParcelFileDescriptor.open(tmpFile, ParcelFileDescriptor.MODE_READ_ONLY);
if (!tmpFile.delete()) { if (!tmpFile.delete()) {
Log.w(TAG, "Failed to delete temp file."); Log.w(TAG, "Failed to delete temp file.");
} }
return pdf; return pdf;
} catch (IOException ioe) { } catch (IOException ioe) {
Log.w(TAG, ioe); Log.w(TAG, ioe);

View file

@ -142,6 +142,7 @@ public class PduPart {
*/ */
public PduPart() { public PduPart() {
mPartHeader = new HashMap<Integer, Object>(); mPartHeader = new HashMap<Integer, Object>();
setContentId(String.valueOf(System.currentTimeMillis()).getBytes());
} }
public void setEncrypted(boolean isEncrypted) { public void setEncrypted(boolean isEncrypted) {