From 23579a9b1d3d589320d5d019e5342e4a7adbb482 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 10 Dec 2020 12:20:10 -0500 Subject: [PATCH] Do not unnecessarily refresh known-unregistered users during migration. --- .../securesms/groups/GroupsV1MigrationUtil.java | 13 ++++++++----- .../securesms/jobs/GroupV1MigrationJob.java | 15 ++++++++++++++- .../securesms/recipients/RecipientUtil.java | 5 ++++- .../thoughtcrime/securesms/util/FeatureFlags.java | 10 +++++++++- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupsV1MigrationUtil.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupsV1MigrationUtil.java index c6b91aa4b..7360b2cba 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupsV1MigrationUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupsV1MigrationUtil.java @@ -96,12 +96,15 @@ public final class GroupsV1MigrationUtil { throw new InvalidMigrationStateException(); } - RecipientUtil.ensureUuidsAreAvailable(context, groupRecipient.getParticipants()); - groupRecipient = groupRecipient.fresh(); - List registeredMembers = RecipientUtil.getEligibleForSending(groupRecipient.getParticipants()); - List possibleMembers = forced ? getMigratableManualMigrationMembers(registeredMembers) - : getMigratableAutoMigrationMembers(registeredMembers); + + if (RecipientUtil.ensureUuidsAreAvailable(context, registeredMembers)) { + Log.i(TAG, "Newly-discovered UUIDs. Getting fresh recipients."); + registeredMembers = Stream.of(registeredMembers).map(Recipient::fresh).toList(); + } + + List possibleMembers = forced ? getMigratableManualMigrationMembers(registeredMembers) + : getMigratableAutoMigrationMembers(registeredMembers); if (!forced && possibleMembers.size() != registeredMembers.size()) { Log.w(TAG, "Not allowed to invite or leave registered users behind in an auto-migration! Skipping."); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV1MigrationJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV1MigrationJob.java index 89225de33..59d799c07 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV1MigrationJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV1MigrationJob.java @@ -61,6 +61,11 @@ public class GroupV1MigrationJob extends BaseJob { } public static void enqueuePossibleAutoMigrate(@NonNull RecipientId recipientId) { + if (!FeatureFlags.groupsV1MigrationJob()) { + Log.w(TAG, "Migration job is disabled."); + return; + } + SignalExecutors.BOUNDED.execute(() -> { if (Recipient.resolved(recipientId).isPushV1Group()) { ApplicationDependencies.getJobManager().add(new GroupV1MigrationJob(recipientId)); @@ -69,6 +74,11 @@ public class GroupV1MigrationJob extends BaseJob { } public static void enqueueRoutineMigrationsIfNecessary(@NonNull Application application) { + if (!FeatureFlags.groupsV1MigrationJob()) { + Log.w(TAG, "Migration job is disabled."); + return; + } + if (!SignalStore.registrationValues().isRegistrationComplete() || !TextSecurePreferences.isPushRegistered(application) || TextSecurePreferences.getLocalUuid(application) == null) @@ -82,12 +92,15 @@ public class GroupV1MigrationJob extends BaseJob { return; } - long timeSinceRefresh = System.currentTimeMillis() - SignalStore.misc().getLastProfileRefreshTime(); + long timeSinceRefresh = System.currentTimeMillis() - SignalStore.misc().getLastGv1RoutineMigrationTime(); + if (timeSinceRefresh < REFRESH_INTERVAL) { Log.i(TAG, "Too soon to refresh. Did the last refresh " + timeSinceRefresh + " ms ago."); return; } + SignalStore.misc().setLastGv1RoutineMigrationTime(System.currentTimeMillis()); + SignalExecutors.BOUNDED.execute(() -> { JobManager jobManager = ApplicationDependencies.getJobManager(); List threads = DatabaseFactory.getThreadDatabase(application).getRecentV1Groups(ROUTINE_LIMIT); diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientUtil.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientUtil.java index 74b9d2970..192c3cdff 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientUtil.java @@ -93,7 +93,7 @@ public class RecipientUtil { .toList(); } - public static void ensureUuidsAreAvailable(@NonNull Context context, @NonNull Collection recipients) + public static boolean ensureUuidsAreAvailable(@NonNull Context context, @NonNull Collection recipients) throws IOException { List recipientsWithoutUuids = Stream.of(recipients) @@ -103,6 +103,9 @@ public class RecipientUtil { if (recipientsWithoutUuids.size() > 0) { DirectoryHelper.refreshDirectoryFor(context, recipientsWithoutUuids, false); + return true; + } else { + return false; } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java index 9e083979f..4eaa6c360 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java @@ -64,6 +64,7 @@ public final class FeatureFlags { private static final String GV1_AUTO_MIGRATE = "android.groupsV1Migration.auto.4"; private static final String GV1_MANUAL_MIGRATE = "android.groupsV1Migration.manual"; private static final String GV1_FORCED_MIGRATE = "android.groupsV1Migration.forced"; + private static final String GV1_MIGRATION_JOB = "android.groupsV1Migration.job"; private static final String SEND_VIEWED_RECEIPTS = "android.sendViewedReceipts"; /** @@ -81,6 +82,7 @@ public final class FeatureFlags { DONATE_MEGAPHONE, VIEWED_RECEIPTS, GV1_AUTO_MIGRATE, + GV1_MIGRATION_JOB, GV1_MANUAL_MIGRATE, GV1_FORCED_MIGRATE, GROUP_CALLING, @@ -107,7 +109,8 @@ public final class FeatureFlags { private static final Set HOT_SWAPPABLE = SetUtil.newHashSet( VERIFY_V2, CLIENT_EXPIRATION, - GROUP_CALLING + GROUP_CALLING, + GV1_MIGRATION_JOB ); /** @@ -238,6 +241,11 @@ public final class FeatureFlags { return getBoolean(GV1_AUTO_MIGRATE, false); } + /** Whether or not we should run the job to proactively migrate groups. */ + public static boolean groupsV1MigrationJob() { + return getBoolean(GV1_MIGRATION_JOB, false); + } + /** Whether or not manual migration from GV1->GV2 is enabled. */ public static boolean groupsV1ManualMigration() { return getBoolean(GV1_MANUAL_MIGRATE, false) && groupsV1AutoMigration();