From c6f29fc9504b6938ba135188d05672eff432febb Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 16 Dec 2022 23:29:07 -0500 Subject: [PATCH] Migrate queued jobs during SMS migration. --- .../conversation/ConversationFragment.java | 2 +- .../V168_SingleMessageTableMigration.kt | 3 +- .../jobmanager/persistence/JobSpec.java | 4 + .../securesms/jobs/JobManagerFactories.java | 2 + .../securesms/jobs/RemoteDeleteSendJob.java | 30 ++------ .../keyvalue/PlainTextSharedPrefsDataStore.kt | 32 ++++++++ .../securesms/keyvalue/SignalStore.java | 7 ++ .../mediapreview/MediaPreviewRepository.kt | 2 +- .../migrations/ApplicationMigrations.java | 7 +- .../migrations/UpdateSmsJobsMigrationJob.kt | 76 +++++++++++++++++++ .../securesms/sms/MessageSender.java | 6 +- .../custom/PrivateStorySettingsRepository.kt | 2 +- .../story/StoriesPrivacySettingsRepository.kt | 2 +- .../securesms/util/DeleteDialog.kt | 2 +- 14 files changed, 144 insertions(+), 33 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/keyvalue/PlainTextSharedPrefsDataStore.kt create mode 100644 app/src/main/java/org/thoughtcrime/securesms/migrations/UpdateSmsJobsMigrationJob.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java index 9e70c0212..e8df3e71a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java @@ -1093,7 +1093,7 @@ public class ConversationFragment extends LoggingFragment implements Multiselect Runnable deleteForEveryone = () -> { SignalExecutors.BOUNDED.execute(() -> { for (MessageRecord message : messageRecords) { - MessageSender.sendRemoteDelete(message.getId(), message.isMms()); + MessageSender.sendRemoteDelete(message.getId()); } }); }; diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt index e1edc2a51..ebdcde471 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V168_SingleMessageTableMigration.kt @@ -5,6 +5,7 @@ import net.zetetic.database.sqlcipher.SQLiteDatabase import org.signal.core.util.SqlUtil import org.signal.core.util.Stopwatch import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.keyvalue.SignalStore object V168_SingleMessageTableMigration : SignalDatabaseMigration { private val TAG = Log.tag(V168_SingleMessageTableMigration::class.java) @@ -140,6 +141,6 @@ object V168_SingleMessageTableMigration : SignalDatabaseMigration { stopwatch.stop(TAG) - // TODO jobs? + SignalStore.plaintext().smsMigrationIdOffset = nextMmsId } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobSpec.java b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobSpec.java index b0e35d6fa..2d4c65955 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobSpec.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobSpec.java @@ -54,6 +54,10 @@ public final class JobSpec { return new JobSpec(id, factoryKey, queueKey, createTime, updated, runAttempt, maxAttempts, lifespan, serializedData, serializedInputData, isRunning, memoryOnly); } + public @NonNull JobSpec withData(String updatedSerializedData) { + return new JobSpec(id, factoryKey, queueKey, createTime, nextRunAttemptTime, runAttempt, maxAttempts, lifespan, updatedSerializedData, serializedInputData, isRunning, memoryOnly); + } + public @NonNull String getId() { return id; } 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 ada6b7e01..50e9d6878 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java @@ -54,6 +54,7 @@ import org.thoughtcrime.securesms.migrations.PniMigrationJob; import org.thoughtcrime.securesms.migrations.ProfileMigrationJob; import org.thoughtcrime.securesms.migrations.ProfileSharingUpdateMigrationJob; import org.thoughtcrime.securesms.migrations.RebuildMessageSearchIndexMigrationJob; +import org.thoughtcrime.securesms.migrations.UpdateSmsJobsMigrationJob; import org.thoughtcrime.securesms.migrations.RecipientSearchMigrationJob; import org.thoughtcrime.securesms.migrations.RegistrationPinV2MigrationJob; import org.thoughtcrime.securesms.migrations.StickerAdditionMigrationJob; @@ -233,6 +234,7 @@ public final class JobManagerFactories { put(StoryReadStateMigrationJob.KEY, new StoryReadStateMigrationJob.Factory()); put(StoryViewedReceiptsStateMigrationJob.KEY, new StoryViewedReceiptsStateMigrationJob.Factory()); put(TrimByLengthSettingsMigrationJob.KEY, new TrimByLengthSettingsMigrationJob.Factory()); + put(UpdateSmsJobsMigrationJob.KEY, new UpdateSmsJobsMigrationJob.Factory()); put(UserNotificationMigrationJob.KEY, new UserNotificationMigrationJob.Factory()); put(UuidMigrationJob.KEY, new UuidMigrationJob.Factory()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java index e0a4e3f8e..ec11cc2ce 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java @@ -45,22 +45,19 @@ public class RemoteDeleteSendJob extends BaseJob { private static final String TAG = Log.tag(RemoteDeleteSendJob.class); private static final String KEY_MESSAGE_ID = "message_id"; - private static final String KEY_IS_MMS = "is_mms"; private static final String KEY_RECIPIENTS = "recipients"; private static final String KEY_INITIAL_RECIPIENT_COUNT = "initial_recipient_count"; private final long messageId; - private final boolean isMms; private final List recipients; private final int initialRecipientCount; @WorkerThread - public static @NonNull JobManager.Chain create(long messageId, boolean isMms) + public static @NonNull JobManager.Chain create(long messageId) throws NoSuchMessageException { - MessageRecord message = isMms ? SignalDatabase.messages().getMessageRecord(messageId) - : SignalDatabase.messages().getSmsMessage(messageId); + MessageRecord message = SignalDatabase.messages().getMessageRecord(messageId); Recipient conversationRecipient = SignalDatabase.threads().getRecipientForThreadId(message.getThreadId()); @@ -82,7 +79,6 @@ public class RemoteDeleteSendJob extends BaseJob { recipients.remove(Recipient.self().getId()); RemoteDeleteSendJob sendJob = new RemoteDeleteSendJob(messageId, - isMms, recipients, recipients.size(), new Parameters.Builder() @@ -101,7 +97,6 @@ public class RemoteDeleteSendJob extends BaseJob { } private RemoteDeleteSendJob(long messageId, - boolean isMms, @NonNull List recipients, int initialRecipientCount, @NonNull Parameters parameters) @@ -109,7 +104,6 @@ public class RemoteDeleteSendJob extends BaseJob { super(parameters); this.messageId = messageId; - this.isMms = isMms; this.recipients = recipients; this.initialRecipientCount = initialRecipientCount; } @@ -117,7 +111,6 @@ public class RemoteDeleteSendJob extends BaseJob { @Override public @NonNull Data serialize() { return new Data.Builder().putLong(KEY_MESSAGE_ID, messageId) - .putBoolean(KEY_IS_MMS, isMms) .putString(KEY_RECIPIENTS, RecipientId.toSerializedList(recipients)) .putInt(KEY_INITIAL_RECIPIENT_COUNT, initialRecipientCount) .build(); @@ -134,18 +127,10 @@ public class RemoteDeleteSendJob extends BaseJob { throw new NotPushRegisteredException(); } - MessageTable db; - MessageRecord message; + MessageTable db = SignalDatabase.messages(); + MessageRecord message = SignalDatabase.messages().getMessageRecord(messageId); - if (isMms) { - db = SignalDatabase.messages(); - message = SignalDatabase.messages().getMessageRecord(messageId); - } else { - db = SignalDatabase.messages(); - message = SignalDatabase.messages().getSmsMessage(messageId); - } - - long targetSentTimestamp = message.getDateSent(); + long targetSentTimestamp = message.getDateSent(); Recipient conversationRecipient = SignalDatabase.threads().getRecipientForThreadId(message.getThreadId()); if (conversationRecipient == null) { @@ -182,7 +167,7 @@ public class RemoteDeleteSendJob extends BaseJob { Log.i(TAG, "Completed now: " + sendResult.completed.size() + ", Skipped: " + totalSkips.size() + ", Remaining: " + recipients.size()); - if (totalSkips.size() > 0 && isMms && message.getRecipient().isGroup()) { + if (totalSkips.size() > 0 && message.getRecipient().isGroup()) { SignalDatabase.groupReceipts().setSkipped(totalSkips, messageId); } @@ -242,11 +227,10 @@ public class RemoteDeleteSendJob extends BaseJob { @Override public @NonNull RemoteDeleteSendJob create(@NonNull Parameters parameters, @NonNull Data data) { long messageId = data.getLong(KEY_MESSAGE_ID); - boolean isMms = data.getBoolean(KEY_IS_MMS); List recipients = RecipientId.fromSerializedList(data.getString(KEY_RECIPIENTS)); int initialRecipientCount = data.getInt(KEY_INITIAL_RECIPIENT_COUNT); - return new RemoteDeleteSendJob(messageId, isMms, recipients, initialRecipientCount, parameters); + return new RemoteDeleteSendJob(messageId, recipients, initialRecipientCount, parameters); } } } \ No newline at end of file diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PlainTextSharedPrefsDataStore.kt b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PlainTextSharedPrefsDataStore.kt new file mode 100644 index 000000000..2c8e3e0e5 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PlainTextSharedPrefsDataStore.kt @@ -0,0 +1,32 @@ +package org.thoughtcrime.securesms.keyvalue + +import android.annotation.SuppressLint +import android.content.Context +import androidx.preference.PreferenceManager + +/** + * There are some values that you can't, for whatever reason, store in the normal encrypted [KeyValueStore]. + * Usually, it's because the value your storing is _related to_ the database. Regardless, this is just a normal + * shared-prefs-backed class. Do not put anything in here that you wouldn't be comfortable storing in plain text. + * + * A good rule of thumb might be: if you're not comforable logging it, then you shouldn't be comfortable putting + * it in here. + */ +class PlainTextSharedPrefsDataStore(private val context: Context) { + + companion object { + const val SMS_MIGRATION_ID_OFFSET = "sms_migration_id_offset" + } + + private val sharedPrefs = PreferenceManager.getDefaultSharedPreferences(context) + + /** + * Stores the ID offset that was determined during the big migration that moved all SMS messages into the MMS table. + */ + var smsMigrationIdOffset: Long + get() = sharedPrefs.getLong(SMS_MIGRATION_ID_OFFSET, -1) + @SuppressLint("ApplySharedPref") + set(value) { + sharedPrefs.edit().putLong(SMS_MIGRATION_ID_OFFSET, value).commit() + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java index 6af50473f..9e1e700b7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java @@ -44,6 +44,8 @@ public final class SignalStore { private final ReleaseChannelValues releaseChannelValues; private final StoryValues storyValues; + private final PlainTextSharedPrefsDataStore plainTextValues; + private static volatile SignalStore instance; private static @NonNull SignalStore getInstance() { @@ -85,6 +87,7 @@ public final class SignalStore { this.notificationProfileValues = new NotificationProfileValues(store); this.releaseChannelValues = new ReleaseChannelValues(store); this.storyValues = new StoryValues(store); + this.plainTextValues = new PlainTextSharedPrefsDataStore(ApplicationDependencies.getApplication()); } public static void onFirstEverAppLaunch() { @@ -269,6 +272,10 @@ public final class SignalStore { return new SignalPreferenceDataStore(getStore()); } + public static @NonNull PlainTextSharedPrefsDataStore plaintext() { + return getInstance().plainTextValues; + } + /** * Ensures any pending writes are finished. Only intended to be called by * {@link SignalUncaughtExceptionHandler}. diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt index d57efa53b..07eab2449 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewRepository.kt @@ -84,7 +84,7 @@ class MediaPreviewRepository { fun remoteDelete(attachment: DatabaseAttachment): Completable { return Completable.fromRunnable { - MessageSender.sendRemoteDelete(attachment.mmsId, true) + MessageSender.sendRemoteDelete(attachment.mmsId) }.subscribeOn(Schedulers.io()) } 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 7e873fa94..950f8071c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -114,9 +114,10 @@ public class ApplicationMigrations { static final int THREAD_MESSAGE_SCHEMA_CHANGE = 70; static final int SMS_MMS_MERGE = 71; static final int REBUILD_MESSAGE_FTS_INDEX = 72; + static final int UPDATE_SMS_JOBS = 73; } - public static final int CURRENT_VERSION = 72; + public static final int CURRENT_VERSION = 73; /** * This *must* be called after the {@link JobManager} has been instantiated, but *before* the call @@ -506,6 +507,10 @@ public class ApplicationMigrations { jobs.put(Version.REBUILD_MESSAGE_FTS_INDEX, new RebuildMessageSearchIndexMigrationJob()); } + if (lastSeenVersion < Version.UPDATE_SMS_JOBS) { + jobs.put(Version.UPDATE_SMS_JOBS, new UpdateSmsJobsMigrationJob()); + } + return jobs; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/UpdateSmsJobsMigrationJob.kt b/app/src/main/java/org/thoughtcrime/securesms/migrations/UpdateSmsJobsMigrationJob.kt new file mode 100644 index 000000000..2149bc668 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/UpdateSmsJobsMigrationJob.kt @@ -0,0 +1,76 @@ +package org.thoughtcrime.securesms.migrations + +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies +import org.thoughtcrime.securesms.jobmanager.Data +import org.thoughtcrime.securesms.jobmanager.Data.Serializer +import org.thoughtcrime.securesms.jobmanager.Job +import org.thoughtcrime.securesms.jobmanager.persistence.JobSpec +import org.thoughtcrime.securesms.keyvalue.SignalStore + +/** + * Updates the data in queued jobs to reflect the new ids SMS messages get assigned during the table merge migration. + * Normally we'd do this in a JobManager migration, but unfortunately this migration requires that a database migration + * happened already, but we don't want the database to be accessed until the [DatabaseMigrationJob] is run, otherwise + * we won't show the progress update. + * + * This ends up being more straightforward regardless because by the time this application migration is being run, it must be the + * case that the database migration is finished (since it's enqueued after the [DatabaseMigrationJob]), so we don't have to + * do any weird wait-notify stuff to guarantee the offset is set. + */ +internal class UpdateSmsJobsMigrationJob( + parameters: Parameters = Parameters.Builder().build() +) : MigrationJob(parameters) { + + companion object { + val TAG = Log.tag(UpdateSmsJobsMigrationJob::class.java) + const val KEY = "UpdateSmsJobsMigrationJob" + } + + override fun getFactoryKey(): String = KEY + + override fun isUiBlocking(): Boolean = false + + override fun performMigration() { + val idOffset = SignalStore.plaintext().smsMigrationIdOffset + check(idOffset >= 0) { "Invalid ID offset of $idOffset -- this shouldn't be possible!" } + + ApplicationDependencies.getJobManager().update { jobSpec, serializer -> + when (jobSpec.factoryKey) { + "PushTextSendJob" -> jobSpec.updateAndSerialize(serializer, "message_id", null, idOffset) + "ReactionSendJob" -> jobSpec.updateAndSerialize(serializer, "message_id", "is_mms", idOffset) + "RemoteDeleteSendJob" -> jobSpec.updateAndSerialize(serializer, "message_id", "is_mms", idOffset) + "SmsSendJob" -> jobSpec.updateAndSerialize(serializer, "message_id", null, idOffset) + "SmsSentJob" -> jobSpec.updateAndSerialize(serializer, "message_id", null, idOffset) + else -> jobSpec + } + } + } + + private fun JobSpec.updateAndSerialize(serializer: Serializer, idKey: String, isMmsKey: String?, offset: Long): JobSpec { + val data = serializer.deserialize(this.serializedData) + + if (isMmsKey != null && data.getBooleanOrDefault(isMmsKey, false)) { + return this + } + + return if (data.hasLong(idKey)) { + val currentValue: Long = data.getLong(idKey) + val updatedValue: Long = currentValue + offset + val updatedData: Data = data.buildUpon().putLong(idKey, updatedValue).build() + + Log.d(TAG, "Updating job with factory ${this.factoryKey} from $currentValue to $updatedValue") + this.withData(serializer.serialize(updatedData)) + } else { + this + } + } + + override fun shouldRetry(e: Exception): Boolean = false + + class Factory : Job.Factory { + override fun create(parameters: Parameters, data: Data): UpdateSmsJobsMigrationJob { + return UpdateSmsJobsMigrationJob(parameters) + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java index 6f2f54954..6e68a2180 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java @@ -466,13 +466,13 @@ public class MessageSender { } } - public static void sendRemoteDelete(long messageId, boolean isMms) { - MessageTable db = isMms ? SignalDatabase.messages() : SignalDatabase.messages(); + public static void sendRemoteDelete(long messageId) { + MessageTable db = SignalDatabase.messages(); db.markAsRemoteDelete(messageId); db.markAsSending(messageId); try { - RemoteDeleteSendJob.create(messageId, isMms).enqueue(); + RemoteDeleteSendJob.create(messageId).enqueue(); onMessageSent(); } catch (NoSuchMessageException e) { Log.w(TAG, "[sendRemoteDelete] Could not find message! Ignoring."); diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/settings/custom/PrivateStorySettingsRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/settings/custom/PrivateStorySettingsRepository.kt index 63c10080c..613ea973a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/settings/custom/PrivateStorySettingsRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/settings/custom/PrivateStorySettingsRepository.kt @@ -32,7 +32,7 @@ class PrivateStorySettingsRepository { val recipientId = SignalDatabase.recipients.getOrInsertFromDistributionListId(distributionListId) SignalDatabase.messages.getAllStoriesFor(recipientId, -1).use { reader -> for (record in reader) { - MessageSender.sendRemoteDelete(record.id, record.isMms) + MessageSender.sendRemoteDelete(record.id) } } }.subscribeOn(Schedulers.io()) diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/settings/story/StoriesPrivacySettingsRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/settings/story/StoriesPrivacySettingsRepository.kt index 96b119dc1..f6a4825a2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/settings/story/StoriesPrivacySettingsRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/settings/story/StoriesPrivacySettingsRepository.kt @@ -32,7 +32,7 @@ class StoriesPrivacySettingsRepository { SignalDatabase.messages.getAllOutgoingStories(false, -1).use { reader -> reader.map { record -> record.id } }.forEach { messageId -> - MessageSender.sendRemoteDelete(messageId, true) + MessageSender.sendRemoteDelete(messageId) } }.subscribeOn(Schedulers.io()) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/DeleteDialog.kt b/app/src/main/java/org/thoughtcrime/securesms/util/DeleteDialog.kt index 82d02c6d8..1b3781753 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/DeleteDialog.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/DeleteDialog.kt @@ -74,7 +74,7 @@ object DeleteDialog { private fun deleteForEveryone(messageRecords: Set, emitter: SingleEmitter) { SignalExecutors.BOUNDED.execute { messageRecords.forEach { message -> - MessageSender.sendRemoteDelete(message.id, message.isMms) + MessageSender.sendRemoteDelete(message.id) } emitter.onSuccess(false)