Skip to content

Commit dc903e4

Browse files
committed
Fix image cache bugs.
1) Index SlideDeckListener cache by MMS (id, timestamp) tuple. 2) Index parts by (id, content_id) tuples. Fixes signalapp#840 Closes signalapp#3183 // FREEBIE
1 parent 0829852 commit dc903e4

8 files changed

Lines changed: 113 additions & 45 deletions

File tree

src/org/thoughtcrime/securesms/database/MmsDatabase.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ public class MmsDatabase extends MessagingDatabase {
165165
};
166166

167167
public static final ExecutorService slideResolver = org.thoughtcrime.securesms.util.Util.newSingleThreadedLifoExecutor();
168-
private static final Map<Long, SoftReference<SlideDeck>> slideCache =
169-
Collections.synchronizedMap(new LRUCache<Long, SoftReference<SlideDeck>>(20));
168+
private static final Map<String, SoftReference<SlideDeck>> slideCache =
169+
Collections.synchronizedMap(new LRUCache<String, SoftReference<SlideDeck>>(20));
170170

171171
private final JobManager jobManager;
172172

@@ -1045,7 +1045,7 @@ private MediaMmsMessageRecord getMediaMmsMessageRecord(Cursor cursor) {
10451045
List<IdentityKeyMismatch> mismatches = getMismatchedIdentities(mismatchDocument);
10461046
List<NetworkFailure> networkFailures = getFailures(networkDocument);
10471047

1048-
ListenableFutureTask<SlideDeck> slideDeck = getSlideDeck(masterSecret, id);
1048+
ListenableFutureTask<SlideDeck> slideDeck = getSlideDeck(masterSecret, dateReceived, id);
10491049

10501050
return new MediaMmsMessageRecord(context, id, recipients, recipients.getPrimaryRecipient(),
10511051
addressDeviceId, dateSent, dateReceived, receiptCount,
@@ -1109,9 +1109,10 @@ private DisplayRecord.Body getBody(Cursor cursor) {
11091109
}
11101110

11111111
private ListenableFutureTask<SlideDeck> getSlideDeck(final MasterSecret masterSecret,
1112+
final long timestamp,
11121113
final long id)
11131114
{
1114-
ListenableFutureTask<SlideDeck> future = getCachedSlideDeck(id);
1115+
ListenableFutureTask<SlideDeck> future = getCachedSlideDeck(timestamp, id);
11151116

11161117
if (future != null) {
11171118
return future;
@@ -1128,21 +1129,21 @@ public SlideDeck call() throws Exception {
11281129
SlideDeck slideDeck = new SlideDeck(context, masterSecret, body);
11291130

11301131
if (!body.containsPushInProgress()) {
1131-
slideCache.put(id, new SoftReference<SlideDeck>(slideDeck));
1132+
slideCache.put(timestamp + "::" + id, new SoftReference<>(slideDeck));
11321133
}
11331134

11341135
return slideDeck;
11351136
}
11361137
};
11371138

1138-
future = new ListenableFutureTask<SlideDeck>(task);
1139+
future = new ListenableFutureTask<>(task);
11391140
slideResolver.execute(future);
11401141

11411142
return future;
11421143
}
11431144

1144-
private ListenableFutureTask<SlideDeck> getCachedSlideDeck(final long id) {
1145-
SoftReference<SlideDeck> reference = slideCache.get(id);
1145+
private ListenableFutureTask<SlideDeck> getCachedSlideDeck(final long timestamp, final long id) {
1146+
SoftReference<SlideDeck> reference = slideCache.get(timestamp + "::" + id);
11461147

11471148
if (reference != null) {
11481149
final SlideDeck slideDeck = reference.get();
@@ -1155,7 +1156,7 @@ public SlideDeck call() throws Exception {
11551156
}
11561157
};
11571158

1158-
ListenableFutureTask<SlideDeck> future = new ListenableFutureTask<SlideDeck>(task);
1159+
ListenableFutureTask<SlideDeck> future = new ListenableFutureTask<>(task);
11591160
future.run();
11601161

11611162
return future;

src/org/thoughtcrime/securesms/database/PartDatabase.java

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ public class PartDatabase extends Database {
7979
private static final String THUMBNAIL = "thumbnail";
8080
private static final String ASPECT_RATIO = "aspect_ratio";
8181

82+
private static final String ID_CONTENT_WHERE = ID + " = ? AND " + CONTENT_ID + " = ?";
83+
8284
public static final String CREATE_TABLE = "CREATE TABLE " + TABLE_NAME + " (" + ID + " INTEGER PRIMARY KEY, " +
8385
MMS_ID + " INTEGER, " + SEQUENCE + " INTEGER DEFAULT 0, " +
8486
CONTENT_TYPE + " TEXT, " + NAME + " TEXT, " + CHARSET + " INTEGER, " +
@@ -113,10 +115,10 @@ public PartDatabase(Context context, SQLiteOpenHelper databaseHelper) {
113115
super(context, databaseHelper);
114116
}
115117

116-
public InputStream getPartStream(MasterSecret masterSecret, long partId)
118+
public InputStream getPartStream(MasterSecret masterSecret, long partId, byte[] contentId)
117119
throws FileNotFoundException
118120
{
119-
return getDataStream(masterSecret, partId, DATA);
121+
return getDataStream(masterSecret, partId, contentId, DATA);
120122
}
121123

122124
public void updateFailedDownloadedPart(long messageId, long partId, PduPart part)
@@ -206,7 +208,7 @@ public void deleteParts(long mmsId) {
206208
cursor.close();
207209
}
208210

209-
database.delete(TABLE_NAME, MMS_ID + " = ?", new String[] {mmsId+""});
211+
database.delete(TABLE_NAME, MMS_ID + " = ?", new String[] {mmsId + ""});
210212
}
211213

212214
@SuppressWarnings("ResultOfMethodCallIgnored")
@@ -343,15 +345,17 @@ protected OutputStream getPartOutputStream(MasterSecret masterSecret, File path,
343345
return new EncryptingPartOutputStream(path, masterSecret);
344346
}
345347

346-
@VisibleForTesting InputStream getDataStream(MasterSecret masterSecret, long partId, String dataType)
348+
@VisibleForTesting InputStream getDataStream(MasterSecret masterSecret, long partId, byte[] contentId, String dataType)
347349
throws FileNotFoundException
348350
{
349351
SQLiteDatabase database = databaseHelper.getReadableDatabase();
350352
Cursor cursor = null;
351353

352354
try {
353-
cursor = database.query(TABLE_NAME, new String[]{dataType}, ID_WHERE,
354-
new String[] {partId+""}, null, null, null);
355+
cursor = database.query(TABLE_NAME, new String[]{dataType}, ID_CONTENT_WHERE,
356+
new String[] {String.valueOf(partId),
357+
Util.toIsoString(contentId)},
358+
null, null, null);
355359

356360
if (cursor != null && cursor.moveToFirst()) {
357361
if (cursor.isNull(0)) {
@@ -402,15 +406,15 @@ private Pair<File, Long> writePartData(MasterSecret masterSecret, PduPart part)
402406
}
403407
}
404408

405-
public InputStream getThumbnailStream(final MasterSecret masterSecret, final long partId) throws IOException {
409+
public InputStream getThumbnailStream(MasterSecret masterSecret, long partId, byte[] contentId) throws IOException {
406410
Log.w(TAG, "getThumbnailStream(" + partId + ")");
407-
final InputStream dataStream = getDataStream(masterSecret, partId, THUMBNAIL);
411+
final InputStream dataStream = getDataStream(masterSecret, partId, contentId, THUMBNAIL);
408412
if (dataStream != null) {
409413
return dataStream;
410414
}
411415

412416
try {
413-
return thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)).get();
417+
return thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, contentId)).get();
414418
} catch (InterruptedException ie) {
415419
throw new AssertionError("interrupted");
416420
} catch (ExecutionException ee) {
@@ -424,7 +428,7 @@ private PduPart getPart(Cursor cursor) {
424428

425429
getPartValues(part, cursor);
426430

427-
part.setDataUri(ContentUris.withAppendedId(PartAuthority.PART_CONTENT_URI, part.getId()));
431+
part.setDataUri(PartAuthority.getPartUri(part.getId(), part.getContentId()));
428432

429433
return part;
430434
}
@@ -454,7 +458,7 @@ private long insertPart(MasterSecret masterSecret, PduPart part, long mmsId, Bit
454458
ThumbnailData data = new ThumbnailData(thumbnail);
455459
updatePartThumbnail(masterSecret, partId, part, data.toDataStream(), data.getAspectRatio());
456460
} else if (!part.isPendingPush()) {
457-
thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId));
461+
thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, part.getContentId()));
458462
}
459463

460464
return partId;
@@ -479,7 +483,7 @@ public void updateDownloadedPart(MasterSecret masterSecret, long messageId,
479483

480484
database.update(TABLE_NAME, values, ID_WHERE, new String[]{partId+""});
481485

482-
thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId));
486+
thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, part.getContentId()));
483487

484488
notifyConversationListeners(DatabaseFactory.getMmsDatabase(context).getThreadIdForMessage(messageId));
485489
}
@@ -534,11 +538,12 @@ public void updatePartThumbnail(MasterSecret masterSecret, long partId, PduPart
534538

535539
public static class ImageRecord {
536540
private long partId;
541+
private byte[] contentId;
537542
private String contentType;
538543
private String address;
539544
private long date;
540545

541-
private ImageRecord(long partId, String contentType, String address, long date) {
546+
private ImageRecord(long partId, byte[] contentId, String contentType, String address, long date) {
542547
this.partId = partId;
543548
this.contentType = contentType;
544549
this.address = address;
@@ -547,6 +552,7 @@ private ImageRecord(long partId, String contentType, String address, long date)
547552

548553
public static ImageRecord from(Cursor cursor) {
549554
return new ImageRecord(cursor.getLong(cursor.getColumnIndexOrThrow(ID)),
555+
Util.toIsoBytes(cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_ID))),
550556
cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_TYPE)),
551557
cursor.getString(cursor.getColumnIndexOrThrow(MmsDatabase.ADDRESS)),
552558
cursor.getLong(cursor.getColumnIndexOrThrow(MmsDatabase.NORMALIZED_DATE_RECEIVED)) * 1000);
@@ -569,22 +575,24 @@ public long getDate() {
569575
}
570576

571577
public Uri getUri() {
572-
return ContentUris.withAppendedId(PartAuthority.PART_CONTENT_URI, getPartId());
578+
return PartAuthority.getPartUri(partId, contentId);
573579
}
574580
}
575581

576582
@VisibleForTesting class ThumbnailFetchCallable implements Callable<InputStream> {
577583
private final MasterSecret masterSecret;
578584
private final long partId;
585+
private final byte[] contentId;
579586

580-
public ThumbnailFetchCallable(MasterSecret masterSecret, long partId) {
587+
public ThumbnailFetchCallable(MasterSecret masterSecret, long partId, byte[] contentId) {
581588
this.masterSecret = masterSecret;
582589
this.partId = partId;
590+
this.contentId = contentId;
583591
}
584592

585593
@Override
586594
public InputStream call() throws Exception {
587-
final InputStream stream = getDataStream(masterSecret, partId, THUMBNAIL);
595+
final InputStream stream = getDataStream(masterSecret, partId, contentId, THUMBNAIL);
588596
if (stream != null) {
589597
return stream;
590598
}
@@ -599,7 +607,8 @@ public InputStream call() throws Exception {
599607
} catch (BitmapDecodingException bde) {
600608
throw new IOException(bde);
601609
}
602-
return getDataStream(masterSecret, partId, THUMBNAIL);
610+
611+
return getDataStream(masterSecret, partId, contentId, THUMBNAIL);
603612
}
604613
}
605614
}

src/org/thoughtcrime/securesms/mms/ImageSlide.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public Uri getThumbnailUri() {
4646
if (!getPart().isPendingPush() && getPart().getDataUri() != null) {
4747
return isDraft()
4848
? getPart().getDataUri()
49-
: PartAuthority.getThumbnailUri(getPart().getId());
49+
: PartAuthority.getThumbnailUri(getPart().getId(), part.getContentId());
5050
}
5151

5252
return null;

src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ public IncomingMediaMessage(RetrieveConf retreived) {
3232
this.body = retreived.getBody();
3333
this.groupId = null;
3434
this.push = false;
35+
36+
for (int i=0;i<body.getPartsNum();i++) {
37+
body.getPart(i).setContentId(String.valueOf(System.currentTimeMillis()).getBytes());
38+
}
3539
}
3640

3741
public IncomingMediaMessage(MasterSecret masterSecret,

src/org/thoughtcrime/securesms/mms/PartAuthority.java

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.thoughtcrime.securesms.database.DatabaseFactory;
1010
import org.thoughtcrime.securesms.database.PartDatabase;
1111
import org.thoughtcrime.securesms.providers.PartProvider;
12+
import org.thoughtcrime.securesms.util.Hex;
1213

1314
import java.io.IOException;
1415
import java.io.InputStream;
@@ -17,8 +18,8 @@ public class PartAuthority {
1718

1819
private static final String PART_URI_STRING = "content://org.thoughtcrime.securesms/part";
1920
private static final String THUMB_URI_STRING = "content://org.thoughtcrime.securesms/thumb";
20-
public static final Uri PART_CONTENT_URI = Uri.parse(PART_URI_STRING);
21-
public static final Uri THUMB_CONTENT_URI = Uri.parse(THUMB_URI_STRING);
21+
private static final Uri PART_CONTENT_URI = Uri.parse(PART_URI_STRING);
22+
private static final Uri THUMB_CONTENT_URI = Uri.parse(THUMB_URI_STRING);
2223

2324
private static final int PART_ROW = 1;
2425
private static final int THUMB_ROW = 2;
@@ -27,8 +28,8 @@ public class PartAuthority {
2728

2829
static {
2930
uriMatcher = new UriMatcher(UriMatcher.NO_MATCH);
30-
uriMatcher.addURI("org.thoughtcrime.securesms", "part/#", PART_ROW);
31-
uriMatcher.addURI("org.thoughtcrime.securesms", "thumb/#", THUMB_ROW);
31+
uriMatcher.addURI("org.thoughtcrime.securesms", "part/*/#", PART_ROW);
32+
uriMatcher.addURI("org.thoughtcrime.securesms", "thumb/*/#", THUMB_ROW);
3233
}
3334

3435
public static InputStream getPartStream(Context context, MasterSecret masterSecret, Uri uri)
@@ -39,20 +40,32 @@ public static InputStream getPartStream(Context context, MasterSecret masterSecr
3940

4041
try {
4142
switch (match) {
42-
case PART_ROW: return partDatabase.getPartStream(masterSecret, ContentUris.parseId(uri));
43-
case THUMB_ROW: return partDatabase.getThumbnailStream(masterSecret, ContentUris.parseId(uri));
44-
default: return context.getContentResolver().openInputStream(uri);
43+
case PART_ROW:
44+
PartUri partUri = new PartUri(uri);
45+
return partDatabase.getPartStream(masterSecret, partUri.getId(), partUri.getContentId());
46+
case THUMB_ROW:
47+
partUri = new PartUri(uri);
48+
return partDatabase.getThumbnailStream(masterSecret, partUri.getId(), partUri.getContentId());
49+
default:
50+
return context.getContentResolver().openInputStream(uri);
4551
}
4652
} catch (SecurityException se) {
4753
throw new IOException(se);
4854
}
4955
}
5056

5157
public static Uri getPublicPartUri(Uri uri) {
52-
return ContentUris.withAppendedId(PartProvider.CONTENT_URI, ContentUris.parseId(uri));
58+
PartUri partUri = new PartUri(uri);
59+
return PartProvider.getContentUri(partUri.getId(), partUri.getContentId());
5360
}
5461

55-
public static Uri getThumbnailUri(long partId) {
56-
return ContentUris.withAppendedId(THUMB_CONTENT_URI, partId);
62+
public static Uri getPartUri(long partId, byte[] contentId) {
63+
Uri uri = Uri.withAppendedPath(PART_CONTENT_URI, Hex.toStringCondensed(contentId));
64+
return ContentUris.withAppendedId(uri, partId);
65+
}
66+
67+
public static Uri getThumbnailUri(long partId, byte[] contentId) {
68+
Uri uri = Uri.withAppendedPath(THUMB_CONTENT_URI, Hex.toStringCondensed(contentId));
69+
return ContentUris.withAppendedId(uri, partId);
5770
}
5871
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package org.thoughtcrime.securesms.mms;
2+
3+
import android.content.ContentUris;
4+
import android.net.Uri;
5+
6+
import org.thoughtcrime.securesms.util.Hex;
7+
8+
import java.io.IOException;
9+
10+
public class PartUri {
11+
12+
private final Uri uri;
13+
14+
public PartUri(Uri uri) {
15+
this.uri = uri;
16+
}
17+
18+
public long getId() {
19+
return ContentUris.parseId(uri);
20+
}
21+
22+
public byte[] getContentId() {
23+
try {
24+
return Hex.fromStringCondensed(uri.getPathSegments().get(1));
25+
} catch (IOException e) {
26+
throw new AssertionError(e);
27+
}
28+
}
29+
30+
}

0 commit comments

Comments
 (0)