From f65f4704c9d00fbf4d5b3b02f629634f7fa3c85a Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Thu, 15 Jul 2021 13:32:29 -0400 Subject: [PATCH] Improve routine around bulk attachment deletion. --- .../database/AttachmentDatabase.java | 7 +-- .../securesms/database/ThreadDatabase.java | 12 ++++- .../database/helpers/SQLCipherOpenHelper.java | 7 ++- .../securesms/jobs/JobManagerFactories.java | 2 + .../migrations/ApplicationMigrations.java | 9 +++- .../AttachmentCleanupMigrationJob.java | 54 +++++++++++++++++++ 6 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/migrations/AttachmentCleanupMigrationJob.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java index 7141b88cae..d7935915d6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java @@ -466,13 +466,12 @@ public class AttachmentDatabase extends Database { public void trimAllAbandonedAttachments() { SQLiteDatabase db = databaseHelper.getWritableDatabase(); String selectAllMmsIds = "SELECT " + MmsDatabase.ID + " FROM " + MmsDatabase.TABLE_NAME; - String selectDataInUse = "SELECT DISTINCT " + DATA + " FROM " + TABLE_NAME + " WHERE " + QUOTE + " = 0 AND (" + MMS_ID + " IN (" + selectAllMmsIds + ") OR " + MMS_ID + " = " + PREUPLOAD_MESSAGE_ID + ")"; - String where = MMS_ID + " NOT IN (" + selectAllMmsIds + ") AND " + DATA + " NOT IN (" + selectDataInUse + ")"; + String where = MMS_ID + " != " + PREUPLOAD_MESSAGE_ID + " AND " + MMS_ID + " NOT IN (" + selectAllMmsIds + ")"; db.delete(TABLE_NAME, where, null); } - public void deleteAbandonedAttachmentFiles() { + public int deleteAbandonedAttachmentFiles() { Set filesOnDisk = new HashSet<>(); Set filesInDb = new HashSet<>(); @@ -495,6 +494,8 @@ public class AttachmentDatabase extends Database { //noinspection ResultOfMethodCallIgnored new File(filePath).delete(); } + + return onDiskButNotInDatabase.size(); } @SuppressWarnings("ResultOfMethodCallIgnored") diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java index 2359749e6c..741912d340 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java @@ -255,6 +255,7 @@ public class ThreadDatabase extends Database { GroupReceiptDatabase groupReceiptDatabase = DatabaseFactory.getGroupReceiptDatabase(context); MmsSmsDatabase mmsSmsDatabase = DatabaseFactory.getMmsSmsDatabase(context); MentionDatabase mentionDatabase = DatabaseFactory.getMentionDatabase(context); + int deletes = 0; try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, new String[] { ID }, null, null, null, null, null)) { while (cursor != null && cursor.moveToNext()) { @@ -269,12 +270,15 @@ public class ThreadDatabase extends Database { attachmentDatabase.trimAllAbandonedAttachments(); groupReceiptDatabase.deleteAbandonedRows(); mentionDatabase.deleteAbandonedMentions(); - attachmentDatabase.deleteAbandonedAttachmentFiles(); + deletes = attachmentDatabase.deleteAbandonedAttachmentFiles(); db.setTransactionSuccessful(); } finally { db.endTransaction(); } + if (deletes > 0) { + Log.i(TAG, "Trim all threads caused " + deletes + " attachments to be deleted."); + } notifyAttachmentListeners(); notifyStickerListeners(); @@ -291,6 +295,7 @@ public class ThreadDatabase extends Database { GroupReceiptDatabase groupReceiptDatabase = DatabaseFactory.getGroupReceiptDatabase(context); MmsSmsDatabase mmsSmsDatabase = DatabaseFactory.getMmsSmsDatabase(context); MentionDatabase mentionDatabase = DatabaseFactory.getMentionDatabase(context); + int deletes = 0; db.beginTransaction(); @@ -300,12 +305,15 @@ public class ThreadDatabase extends Database { attachmentDatabase.trimAllAbandonedAttachments(); groupReceiptDatabase.deleteAbandonedRows(); mentionDatabase.deleteAbandonedMentions(); - attachmentDatabase.deleteAbandonedAttachmentFiles(); + deletes = attachmentDatabase.deleteAbandonedAttachmentFiles(); db.setTransactionSuccessful(); } finally { db.endTransaction(); } + if (deletes > 0) { + Log.i(TAG, "Trim thread " + threadId + " caused " + deletes + " attachments to be deleted."); + } notifyAttachmentListeners(); notifyStickerListeners(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java index dc7e0bacdd..decf51ced6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java @@ -206,8 +206,9 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper implements SignalDatab private static final int ABANDONED_MESSAGE_CLEANUP = 107; private static final int THREAD_AUTOINCREMENT = 108; private static final int MMS_AUTOINCREMENT = 109; + private static final int ABANDONED_ATTACHMENT_CLEANUP = 110; - private static final int DATABASE_VERSION = 109; + private static final int DATABASE_VERSION = 110; private static final String DATABASE_NAME = "signal.db"; private final Context context; @@ -1929,6 +1930,10 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper implements SignalDatab smsStopwatch.stop(TAG); } + if (oldVersion < ABANDONED_ATTACHMENT_CLEANUP) { + db.delete("part", "mid != -8675309 AND mid NOT IN (SELECT _id FROM mms)", null); + } + db.setTransactionSuccessful(); } finally { db.endTransaction(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java index 579c57fef2..1eb999fd91 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java @@ -31,6 +31,7 @@ import org.thoughtcrime.securesms.jobmanager.migrations.RetrieveProfileJobMigrat import org.thoughtcrime.securesms.jobmanager.migrations.SendReadReceiptsJobMigration; import org.thoughtcrime.securesms.migrations.AccountRecordMigrationJob; import org.thoughtcrime.securesms.migrations.ApplyUnknownFieldsToSelfMigrationJob; +import org.thoughtcrime.securesms.migrations.AttachmentCleanupMigrationJob; import org.thoughtcrime.securesms.migrations.AttributesMigrationJob; import org.thoughtcrime.securesms.migrations.AvatarIdRemovalMigrationJob; import org.thoughtcrime.securesms.migrations.AvatarMigrationJob; @@ -166,6 +167,7 @@ public final class JobManagerFactories { // Migrations put(AccountRecordMigrationJob.KEY, new AccountRecordMigrationJob.Factory()); put(ApplyUnknownFieldsToSelfMigrationJob.KEY, new ApplyUnknownFieldsToSelfMigrationJob.Factory()); + put(AttachmentCleanupMigrationJob.KEY, new AttachmentCleanupMigrationJob.Factory()); put(AttributesMigrationJob.KEY, new AttributesMigrationJob.Factory()); put(AvatarIdRemovalMigrationJob.KEY, new AvatarIdRemovalMigrationJob.Factory()); put(AvatarMigrationJob.KEY, new AvatarMigrationJob.Factory()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java index 5086ab0b96..90ab7807f9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -77,9 +77,10 @@ public class ApplicationMigrations { static final int SENDER_KEY = 35; static final int SENDER_KEY_2 = 36; static final int DB_AUTOINCREMENT = 37; + static final int ATTACHMENT_CLEANUP = 38; } - public static final int CURRENT_VERSION = 37; + public static final int CURRENT_VERSION = 38; /** * This *must* be called after the {@link JobManager} has been instantiated, but *before* the call @@ -322,7 +323,7 @@ public class ApplicationMigrations { } if (lastSeenVersion < Version.APPLY_UNIVERSAL_EXPIRE) { - jobs.put(Version.SMS_STORAGE_SYNC, new ApplyUnknownFieldsToSelfMigrationJob()); + jobs.put(Version.APPLY_UNIVERSAL_EXPIRE, new ApplyUnknownFieldsToSelfMigrationJob()); } if (lastSeenVersion < Version.SENDER_KEY) { @@ -337,6 +338,10 @@ public class ApplicationMigrations { jobs.put(Version.DB_AUTOINCREMENT, new DatabaseMigrationJob()); } + if (lastSeenVersion < Version.ATTACHMENT_CLEANUP) { + jobs.put(Version.ATTACHMENT_CLEANUP, new AttachmentCleanupMigrationJob()); + } + return jobs; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/AttachmentCleanupMigrationJob.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/AttachmentCleanupMigrationJob.java new file mode 100644 index 0000000000..4e0bbddf3b --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/AttachmentCleanupMigrationJob.java @@ -0,0 +1,54 @@ +package org.thoughtcrime.securesms.migrations; + +import androidx.annotation.NonNull; + +import org.signal.core.util.logging.Log; +import org.thoughtcrime.securesms.database.DatabaseFactory; +import org.thoughtcrime.securesms.jobmanager.Data; +import org.thoughtcrime.securesms.jobmanager.Job; + +/** + * Check for abandoned attachments and delete them. + */ +public class AttachmentCleanupMigrationJob extends MigrationJob { + + private static final String TAG = Log.tag(AttachmentCleanupMigrationJob.class); + + public static final String KEY = "AttachmentCleanupMigrationJob"; + + AttachmentCleanupMigrationJob() { + this(new Parameters.Builder().build()); + } + + private AttachmentCleanupMigrationJob(@NonNull Parameters parameters) { + super(parameters); + } + + @Override + public boolean isUiBlocking() { + return false; + } + + @Override + public @NonNull String getFactoryKey() { + return KEY; + } + + @Override + public void performMigration() { + int deletes = DatabaseFactory.getAttachmentDatabase(context).deleteAbandonedAttachmentFiles(); + Log.i(TAG, "Deleted " + deletes + " abandoned attachments."); + } + + @Override + boolean shouldRetry(@NonNull Exception e) { + return false; + } + + public static class Factory implements Job.Factory { + @Override + public @NonNull AttachmentCleanupMigrationJob create(@NonNull Parameters parameters, @NonNull Data data) { + return new AttachmentCleanupMigrationJob(parameters); + } + } +}