From 4e7b4da94108d311108e6cafc9ad2005bbfa1d3e Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Sat, 28 Dec 2019 10:08:11 -0500 Subject: [PATCH] Implement resumable downloads. --- .../api/SignalServiceMessageReceiver.java | 3 +- .../internal/push/PushServiceSocket.java | 28 ++++++--- .../database/AttachmentDatabase.java | 63 +++++++++++++++---- .../database/helpers/SQLCipherOpenHelper.java | 7 ++- .../securesms/jobmanager/JobController.java | 1 + .../securesms/jobs/AttachmentDownloadJob.java | 19 ++---- .../CachedAttachmentsMigrationJob.java | 8 +-- .../migrations/LegacyMigrationJob.java | 14 +---- .../securesms/util/FileUtils.java | 6 +- 9 files changed, 92 insertions(+), 57 deletions(-) diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageReceiver.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageReceiver.java index 9dda99656b..e1d8162506 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageReceiver.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageReceiver.java @@ -134,7 +134,8 @@ public class SignalServiceMessageReceiver { * * @param pointer The {@link SignalServiceAttachmentPointer} * received in a {@link SignalServiceDataMessage}. - * @param destination The download destination for this attachment. + * @param destination The download destination for this attachment. If this file exists, it is + * assumed that this is previously-downloaded content that can be resumed. * @param listener An optional listener (may be null) to receive callbacks on download progress. * * @return An InputStream that streams the plaintext attachment contents. diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java index 7c93e97eca..43f28e5b78 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java @@ -506,7 +506,7 @@ public class PushServiceSocket { String hexPackId = Hex.toStringCondensed(packId); ByteArrayOutputStream output = new ByteArrayOutputStream(); - downloadFromCdn(output, String.format(Locale.US, STICKER_PATH, hexPackId, stickerId), 1024 * 1024, null); + downloadFromCdn(output, 0, String.format(Locale.US, STICKER_PATH, hexPackId, stickerId), 1024 * 1024, null); return output.toByteArray(); } @@ -517,7 +517,7 @@ public class PushServiceSocket { String hexPackId = Hex.toStringCondensed(packId); ByteArrayOutputStream output = new ByteArrayOutputStream(); - downloadFromCdn(output, String.format(STICKER_MANIFEST_PATH, hexPackId), 1024 * 1024, null); + downloadFromCdn(output, 0, String.format(STICKER_MANIFEST_PATH, hexPackId), 1024 * 1024, null); return output.toByteArray(); } @@ -771,14 +771,14 @@ public class PushServiceSocket { private void downloadFromCdn(File destination, String path, int maxSizeBytes, ProgressListener listener) throws PushNetworkException, NonSuccessfulResponseCodeException { - try (FileOutputStream outputStream = new FileOutputStream(destination)) { - downloadFromCdn(outputStream, path, maxSizeBytes, listener); + try (FileOutputStream outputStream = new FileOutputStream(destination, true)) { + downloadFromCdn(outputStream, destination.length(), path, maxSizeBytes, listener); } catch (IOException e) { throw new PushNetworkException(e); } } - private void downloadFromCdn(OutputStream outputStream, String path, int maxSizeBytes, ProgressListener listener) + private void downloadFromCdn(OutputStream outputStream, long offset, String path, int maxSizeBytes, ProgressListener listener) throws PushNetworkException, NonSuccessfulResponseCodeException { ConnectionHolder connectionHolder = getRandom(cdnClients, random); @@ -794,19 +794,25 @@ public class PushServiceSocket { request.addHeader("Host", connectionHolder.getHostHeader().get()); } + if (offset > 0) { + Log.i(TAG, "Starting download from CDN with offset " + offset); + request.addHeader("Range", "bytes=" + offset + "-"); + } + Call call = okHttpClient.newCall(request.build()); synchronized (connections) { connections.add(call); } - Response response; + Response response = null; + ResponseBody body = null; try { response = call.execute(); if (response.isSuccessful()) { - ResponseBody body = response.body(); + body = response.body(); if (body == null) throw new PushNetworkException("No response body!"); if (body.contentLength() > maxSizeBytes) throw new PushNetworkException("Response exceeds max size!"); @@ -814,20 +820,24 @@ public class PushServiceSocket { InputStream in = body.byteStream(); byte[] buffer = new byte[32768]; - int read, totalRead = 0; + int read = 0; + long totalRead = offset; while ((read = in.read(buffer, 0, buffer.length)) != -1) { outputStream.write(buffer, 0, read); if ((totalRead += read) > maxSizeBytes) throw new PushNetworkException("Response exceeded max size!"); if (listener != null) { - listener.onAttachmentProgress(body.contentLength(), totalRead); + listener.onAttachmentProgress(body.contentLength() + offset, totalRead); } } return; } } catch (IOException e) { + if (body != null) { + body.close(); + } throw new PushNetworkException(e); } finally { synchronized (connections) { diff --git a/src/org/thoughtcrime/securesms/database/AttachmentDatabase.java b/src/org/thoughtcrime/securesms/database/AttachmentDatabase.java index 8ca7751e70..e5df073c34 100644 --- a/src/org/thoughtcrime/securesms/database/AttachmentDatabase.java +++ b/src/org/thoughtcrime/securesms/database/AttachmentDatabase.java @@ -57,6 +57,7 @@ import org.thoughtcrime.securesms.stickers.StickerLocator; import org.thoughtcrime.securesms.util.Base64; import org.thoughtcrime.securesms.util.BitmapDecodingException; import org.thoughtcrime.securesms.util.BitmapUtil; +import org.thoughtcrime.securesms.util.FileUtils; import org.thoughtcrime.securesms.util.JsonUtils; import org.thoughtcrime.securesms.util.MediaMetadataRetrieverUtil; import org.thoughtcrime.securesms.util.MediaUtil; @@ -98,6 +99,7 @@ public class AttachmentDatabase extends Database { static final String CONTENT_LOCATION = "cl"; public static final String DATA = "_data"; static final String TRANSFER_STATE = "pending_push"; + private static final String TRANSFER_FILE = "transfer_file"; public static final String SIZE = "data_size"; static final String FILE_NAME = "file_name"; public static final String THUMBNAIL = "thumbnail"; @@ -136,7 +138,7 @@ public class AttachmentDatabase extends Database { UNIQUE_ID, DIGEST, FAST_PREFLIGHT_ID, VOICE_NOTE, QUOTE, DATA_RANDOM, THUMBNAIL_RANDOM, WIDTH, HEIGHT, CAPTION, STICKER_PACK_ID, STICKER_PACK_KEY, STICKER_ID, - DATA_HASH, BLUR_HASH, TRANSFORM_PROPERTIES}; + DATA_HASH, BLUR_HASH, TRANSFORM_PROPERTIES, TRANSFER_FILE }; public static final String CREATE_TABLE = "CREATE TABLE " + TABLE_NAME + " (" + ROW_ID + " INTEGER PRIMARY KEY, " + MMS_ID + " INTEGER, " + "seq" + " INTEGER DEFAULT 0, " + @@ -152,7 +154,7 @@ public class AttachmentDatabase extends Database { CAPTION + " TEXT DEFAULT NULL, " + STICKER_PACK_ID + " TEXT DEFAULT NULL, " + STICKER_PACK_KEY + " DEFAULT NULL, " + STICKER_ID + " INTEGER DEFAULT -1, " + DATA_HASH + " TEXT DEFAULT NULL, " + BLUR_HASH + " TEXT DEFAULT NULL, " + - TRANSFORM_PROPERTIES + " TEXT DEFAULT NULL);"; + TRANSFORM_PROPERTIES + " TEXT DEFAULT NULL, " + TRANSFER_FILE + " TEXT DEFAULT NULL);"; public static final String[] CREATE_INDEXS = { "CREATE INDEX IF NOT EXISTS part_mms_id_index ON " + TABLE_NAME + " (" + MMS_ID + ");", @@ -396,12 +398,7 @@ public class AttachmentDatabase extends Database { SQLiteDatabase database = databaseHelper.getWritableDatabase(); database.delete(TABLE_NAME, null, null); - File attachmentsDirectory = context.getDir(DIRECTORY, Context.MODE_PRIVATE); - File[] attachments = attachmentsDirectory.listFiles(); - - for (File attachment : attachments) { - attachment.delete(); - } + FileUtils.deleteDirectoryContents(context.getDir(DIRECTORY, Context.MODE_PRIVATE)); notifyAttachmentListeners(); } @@ -449,11 +446,12 @@ public class AttachmentDatabase extends Database { public void insertAttachmentsForPlaceholder(long mmsId, @NonNull AttachmentId attachmentId, @NonNull InputStream inputStream) throws MmsException { - DatabaseAttachment placeholder = getAttachment(attachmentId); - SQLiteDatabase database = databaseHelper.getWritableDatabase(); - ContentValues values = new ContentValues(); - DataInfo oldInfo = getAttachmentDataFileInfo(attachmentId, DATA); - DataInfo dataInfo = setAttachmentData(inputStream, false, attachmentId); + DatabaseAttachment placeholder = getAttachment(attachmentId); + SQLiteDatabase database = databaseHelper.getWritableDatabase(); + ContentValues values = new ContentValues(); + DataInfo oldInfo = getAttachmentDataFileInfo(attachmentId, DATA); + DataInfo dataInfo = setAttachmentData(inputStream, false, attachmentId); + File transferFile = getTransferFile(databaseHelper.getReadableDatabase(), attachmentId); if (oldInfo != null) { updateAttachmentDataHash(database, oldInfo.hash, dataInfo); @@ -479,10 +477,16 @@ public class AttachmentDatabase extends Database { values.put(DIGEST, (byte[])null); values.put(NAME, (String) null); values.put(FAST_PREFLIGHT_ID, (String)null); + values.put(TRANSFER_FILE, (String)null); if (database.update(TABLE_NAME, values, PART_ID_WHERE, attachmentId.toStrings()) == 0) { //noinspection ResultOfMethodCallIgnored dataInfo.file.delete(); + + if (transferFile != null) { + //noinspection ResultOfMethodCallIgnored + transferFile.delete(); + } } else { notifyConversationListeners(DatabaseFactory.getMmsDatabase(context).getThreadIdForMessage(mmsId)); notifyConversationListListeners(); @@ -616,6 +620,38 @@ public class AttachmentDatabase extends Database { Log.i(TAG, "[markAttachmentAsTransformed] Updated " + updateCount + " rows."); } + public @NonNull File getOrCreateTransferFile(@NonNull AttachmentId attachmentId) throws IOException { + SQLiteDatabase db = databaseHelper.getWritableDatabase(); + File existing = getTransferFile(db, attachmentId); + + if (existing != null) { + return existing; + } + + File partsDirectory = context.getDir(DIRECTORY, Context.MODE_PRIVATE); + File transferFile = File.createTempFile("transfer", ".mms", partsDirectory); + + ContentValues values = new ContentValues(); + values.put(TRANSFER_FILE, transferFile.getAbsolutePath()); + + db.update(TABLE_NAME, values, PART_ID_WHERE, attachmentId.toStrings()); + + return transferFile; + } + + private @Nullable static File getTransferFile(@NonNull SQLiteDatabase db, @NonNull AttachmentId attachmentId) { + try (Cursor cursor = db.query(TABLE_NAME, new String[] { TRANSFER_FILE }, PART_ID_WHERE, attachmentId.toStrings(), null, null, "1")) { + if (cursor != null && cursor.moveToFirst()) { + String path = cursor.getString(cursor.getColumnIndexOrThrow(TRANSFER_FILE)); + if (path != null) { + return new File(path); + } + } + } + + return null; + } + private static int updateAttachmentAndMatchingHashes(@NonNull SQLiteDatabase database, @NonNull AttachmentId attachmentId, @Nullable String dataHash, @@ -1188,6 +1224,7 @@ public class AttachmentDatabase extends Database { this.hash = hash; } } + public static final class TransformProperties { @JsonProperty private final boolean skipTransform; diff --git a/src/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java b/src/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java index 25a688d0ca..f35b3198ec 100644 --- a/src/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java +++ b/src/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java @@ -98,8 +98,9 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper { private static final int REACTIONS = 37; private static final int STORAGE_SERVICE = 38; private static final int REACTIONS_UNREAD_INDEX = 39; + private static final int RESUMABLE_DOWNLOADS = 40; - private static final int DATABASE_VERSION = 39; + private static final int DATABASE_VERSION = 40; private static final String DATABASE_NAME = "signal.db"; private final Context context; @@ -679,6 +680,10 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper { db.execSQL("CREATE INDEX IF NOT EXISTS mms_reactions_unread_index ON mms (reactions_unread);"); } + if (oldVersion < RESUMABLE_DOWNLOADS) { + db.execSQL("ALTER TABLE part ADD COLUMN transfer_file TEXT DEFAULT NULL"); + } + db.setTransactionSuccessful(); } finally { db.endTransaction(); diff --git a/src/org/thoughtcrime/securesms/jobmanager/JobController.java b/src/org/thoughtcrime/securesms/jobmanager/JobController.java index 5b35d0300f..c24fbe5860 100644 --- a/src/org/thoughtcrime/securesms/jobmanager/JobController.java +++ b/src/org/thoughtcrime/securesms/jobmanager/JobController.java @@ -355,6 +355,7 @@ class JobController { .setMaxAttempts(jobSpec.getMaxAttempts()) .setQueue(jobSpec.getQueueKey()) .setConstraints(Stream.of(constraintSpecs).map(ConstraintSpec::getFactoryKey).toList()) + .setMaxBackoff(jobSpec.getMaxBackoff()) .build(); } diff --git a/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java b/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java index d80df13563..f5a5d3db0b 100644 --- a/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java +++ b/src/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.java @@ -33,6 +33,7 @@ import org.whispersystems.signalservice.api.push.exceptions.PushNetworkException import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.util.concurrent.TimeUnit; public class AttachmentDownloadJob extends BaseJob { @@ -55,12 +56,12 @@ public class AttachmentDownloadJob extends BaseJob { this(new Job.Parameters.Builder() .setQueue("AttachmentDownloadJob" + attachmentId.getRowId() + "-" + attachmentId.getUniqueId()) .addConstraint(NetworkConstraint.KEY) - .setMaxAttempts(25) + .setLifespan(TimeUnit.DAYS.toMillis(1)) + .setMaxAttempts(Parameters.UNLIMITED) .build(), messageId, attachmentId, manual); - } private AttachmentDownloadJob(@NonNull Job.Parameters parameters, long messageId, AttachmentId attachmentId, boolean manual) { @@ -156,11 +157,9 @@ public class AttachmentDownloadJob extends BaseJob { { AttachmentDatabase database = DatabaseFactory.getAttachmentDatabase(context); - File attachmentFile = null; + File attachmentFile = database.getOrCreateTransferFile(attachmentId); try { - attachmentFile = createTempFile(); - SignalServiceMessageReceiver messageReceiver = ApplicationDependencies.getSignalServiceMessageReceiver(); SignalServiceAttachmentPointer pointer = createAttachmentPointer(attachment); InputStream stream = messageReceiver.retrieveAttachment(pointer, attachmentFile, MAX_ATTACHMENT_SIZE, (total, progress) -> EventBus.getDefault().postSticky(new PartProgressEvent(attachment, PartProgressEvent.Type.NETWORK, total, progress))); @@ -169,18 +168,10 @@ public class AttachmentDownloadJob extends BaseJob { } catch (InvalidPartException | NonSuccessfulResponseCodeException | InvalidMessageException | MmsException e) { Log.w(TAG, "Experienced exception while trying to download an attachment.", e); markFailed(messageId, attachmentId); - } finally { - if (attachmentFile != null) { - //noinspection ResultOfMethodCallIgnored - attachmentFile.delete(); - } } } - @VisibleForTesting - SignalServiceAttachmentPointer createAttachmentPointer(Attachment attachment) - throws InvalidPartException - { + private SignalServiceAttachmentPointer createAttachmentPointer(Attachment attachment) throws InvalidPartException { if (TextUtils.isEmpty(attachment.getLocation())) { throw new InvalidPartException("empty content id"); } diff --git a/src/org/thoughtcrime/securesms/migrations/CachedAttachmentsMigrationJob.java b/src/org/thoughtcrime/securesms/migrations/CachedAttachmentsMigrationJob.java index 5a331cae78..b5c0af7eb9 100644 --- a/src/org/thoughtcrime/securesms/migrations/CachedAttachmentsMigrationJob.java +++ b/src/org/thoughtcrime/securesms/migrations/CachedAttachmentsMigrationJob.java @@ -39,12 +39,8 @@ public class CachedAttachmentsMigrationJob extends MigrationJob { return; } - try { - FileUtils.deleteDirectoryContents(context.getExternalCacheDir()); - GlideApp.get(context).clearDiskCache(); - } catch (IOException e) { - Log.w(TAG, e); - } + FileUtils.deleteDirectoryContents(context.getExternalCacheDir()); + GlideApp.get(context).clearDiskCache(); } @Override diff --git a/src/org/thoughtcrime/securesms/migrations/LegacyMigrationJob.java b/src/org/thoughtcrime/securesms/migrations/LegacyMigrationJob.java index 75925446d2..d275a381b8 100644 --- a/src/org/thoughtcrime/securesms/migrations/LegacyMigrationJob.java +++ b/src/org/thoughtcrime/securesms/migrations/LegacyMigrationJob.java @@ -207,20 +207,12 @@ public class LegacyMigrationJob extends MigrationJob { } if (lastSeenVersion < REMOVE_CACHE) { - try { - FileUtils.deleteDirectoryContents(context.getCacheDir()); - } catch (IOException e) { - Log.w(TAG, e); - } + FileUtils.deleteDirectoryContents(context.getCacheDir()); } if (lastSeenVersion < IMAGE_CACHE_CLEANUP) { - try { - FileUtils.deleteDirectoryContents(context.getExternalCacheDir()); - GlideApp.get(context).clearDiskCache(); - } catch (IOException e) { - Log.w(TAG, e); - } + FileUtils.deleteDirectoryContents(context.getExternalCacheDir()); + GlideApp.get(context).clearDiskCache(); } // This migration became unnecessary after switching away from WorkManager diff --git a/src/org/thoughtcrime/securesms/util/FileUtils.java b/src/org/thoughtcrime/securesms/util/FileUtils.java index 7b516a9f9d..1937296c49 100644 --- a/src/org/thoughtcrime/securesms/util/FileUtils.java +++ b/src/org/thoughtcrime/securesms/util/FileUtils.java @@ -1,5 +1,7 @@ package org.thoughtcrime.securesms.util; +import androidx.annotation.Nullable; + import java.io.File; import java.io.FileDescriptor; import java.io.FileInputStream; @@ -34,7 +36,7 @@ public final class FileUtils { } } - public static void deleteDirectoryContents(File directory) throws IOException { + public static void deleteDirectoryContents(@Nullable File directory) { if (directory == null || !directory.exists() || !directory.isDirectory()) return; File[] files = directory.listFiles(); @@ -47,7 +49,7 @@ public final class FileUtils { } } - public static void deleteDirectory(File directory) throws IOException { + public static void deleteDirectory(@Nullable File directory) { if (directory == null || !directory.exists() || !directory.isDirectory()) { return; }