From 817f1ee938c1d044fe3c85c5f9d9c07dc9b73221 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 18 Jun 2021 10:45:00 -0400 Subject: [PATCH] Add a feature flag to disable SMS megaphone. As part of this work, we also make sure we fetch feature flags during registration. --- .../securesms/keyvalue/OnboardingValues.java | 3 +- .../fragments/EnterCodeFragment.java | 22 +++++++++++--- .../fragments/RegistrationLockFragment.java | 30 ++++++++++++++----- .../securesms/util/FeatureFlags.java | 27 +++++++++++++---- .../securesms/util/LocaleFeatureFlags.java | 14 +++++++++ 5 files changed, 77 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/OnboardingValues.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/OnboardingValues.java index 1ffe8725e3..3c0eece3cf 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/OnboardingValues.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/OnboardingValues.java @@ -5,6 +5,7 @@ import android.content.Context; import androidx.annotation.NonNull; import org.thoughtcrime.securesms.phonenumbers.PhoneNumberFormatter; +import org.thoughtcrime.securesms.util.LocaleFeatureFlags; import org.thoughtcrime.securesms.util.Util; import java.util.Collections; @@ -69,7 +70,7 @@ public final class OnboardingValues extends SignalStoreValues { } public boolean shouldShowSms(@NonNull Context context) { - return getBoolean(SHOW_SMS, false) && !Util.isDefaultSmsProvider(context) && PhoneNumberFormatter.getLocalCountryCode() != 91; + return getBoolean(SHOW_SMS, false) && !Util.isDefaultSmsProvider(context) && LocaleFeatureFlags.shouldSuggestSms(); } public void setShowAppearance(boolean value) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/EnterCodeFragment.java b/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/EnterCodeFragment.java index 469498d891..65615e0141 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/EnterCodeFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/EnterCodeFragment.java @@ -31,9 +31,12 @@ import org.thoughtcrime.securesms.registration.service.RegistrationCodeRequest; import org.thoughtcrime.securesms.registration.service.RegistrationService; import org.thoughtcrime.securesms.registration.viewmodel.RegistrationViewModel; import org.thoughtcrime.securesms.util.CommunicationActions; +import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.SupportEmailUtil; import org.thoughtcrime.securesms.util.concurrent.AssertedSuccessListener; +import org.thoughtcrime.securesms.util.concurrent.SimpleTask; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -128,11 +131,22 @@ public final class EnterCodeFragment extends BaseRegistrationFragment @Override public void onSuccessfulRegistration() { - keyboard.displaySuccess().addListener(new AssertedSuccessListener() { - @Override - public void onSuccess(Boolean result) { - handleSuccessfulRegistration(); + SimpleTask.run(() -> { + long startTime = System.currentTimeMillis(); + try { + FeatureFlags.refreshSync(); + Log.i(TAG, "Took " + (System.currentTimeMillis() - startTime) + " ms to get feature flags."); + } catch (IOException e) { + Log.w(TAG, "Failed to refresh flags after " + (System.currentTimeMillis() - startTime) + " ms.", e); } + return null; + }, none -> { + keyboard.displaySuccess().addListener(new AssertedSuccessListener() { + @Override + public void onSuccess(Boolean result) { + handleSuccessfulRegistration(); + } + }); }); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/RegistrationLockFragment.java b/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/RegistrationLockFragment.java index 2596cf193b..ae1f5cf26a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/RegistrationLockFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/fragments/RegistrationLockFragment.java @@ -23,6 +23,7 @@ import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.jobs.StorageAccountRestoreJob; +import org.thoughtcrime.securesms.jobs.StorageSyncJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.lock.v2.PinKeyboardType; import org.thoughtcrime.securesms.pin.PinRestoreRepository.TokenData; @@ -30,10 +31,13 @@ import org.thoughtcrime.securesms.registration.service.CodeVerificationRequest; import org.thoughtcrime.securesms.registration.service.RegistrationService; import org.thoughtcrime.securesms.registration.viewmodel.RegistrationViewModel; import org.thoughtcrime.securesms.util.CommunicationActions; +import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.ServiceUtil; +import org.thoughtcrime.securesms.util.Stopwatch; import org.thoughtcrime.securesms.util.SupportEmailUtil; import org.thoughtcrime.securesms.util.concurrent.SimpleTask; +import java.io.IOException; import java.util.concurrent.TimeUnit; public final class RegistrationLockFragment extends BaseRegistrationFragment { @@ -310,18 +314,28 @@ public final class RegistrationLockFragment extends BaseRegistrationFragment { private void handleSuccessfulPinEntry() { SignalStore.pinValues().setKeyboardType(getPinEntryKeyboardType()); - long startTime = System.currentTimeMillis(); SimpleTask.run(() -> { SignalStore.onboarding().clearAll(); - return ApplicationDependencies.getJobManager().runSynchronously(new StorageAccountRestoreJob(), StorageAccountRestoreJob.LIFESPAN); - }, result -> { - long elapsedTime = System.currentTimeMillis() - startTime; - if (result.isPresent()) { - Log.i(TAG, "Storage Service account restore completed: " + result.get().name() + ". (Took " + elapsedTime + " ms)"); - } else { - Log.i(TAG, "Storage Service account restore failed to complete in the allotted time. (" + elapsedTime + " ms elapsed)"); + Stopwatch stopwatch = new Stopwatch("RegistrationLockRestore"); + + ApplicationDependencies.getJobManager().runSynchronously(new StorageAccountRestoreJob(), StorageAccountRestoreJob.LIFESPAN); + stopwatch.split("AccountRestore"); + + ApplicationDependencies.getJobManager().runSynchronously(new StorageSyncJob(), TimeUnit.SECONDS.toMillis(10)); + stopwatch.split("ContactRestore"); + + try { + FeatureFlags.refreshSync(); + } catch (IOException e) { + Log.w(TAG, "Failed to refresh flags.", e); } + stopwatch.split("FeatureFlags"); + + stopwatch.stop(TAG); + + return null; + }, none -> { cancelSpinning(pinButton); Navigation.findNavController(requireView()).navigate(RegistrationLockFragmentDirections.actionSuccessfulRegistration()); }); 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 9d9e745644..d80b2feadb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java @@ -3,8 +3,8 @@ package org.thoughtcrime.securesms.util; import android.text.TextUtils; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import androidx.annotation.WorkerThread; import com.annimon.stream.Stream; @@ -19,6 +19,7 @@ import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.messageprocessingalarm.MessageProcessReceiver; +import java.io.IOException; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -43,7 +44,7 @@ import java.util.concurrent.TimeUnit; * Other interesting things you can do: * - Make a flag {@link #HOT_SWAPPABLE} * - Make a flag {@link #STICKY} -- booleans only! - * - Register a listener for flag changes in {@link #FLAG_CHANGE_LISTENERS} + * - Register a listener for flag changes in {@link #FLAG_CHANGE_LISTENERS} */ public final class FeatureFlags { @@ -78,6 +79,7 @@ public final class FeatureFlags { private static final String RETRY_RECEIPT_LIFESPAN = "android.retryReceiptLifespan"; private static final String RETRY_RESPOND_MAX_AGE = "android.retryRespondMaxAge"; private static final String SENDER_KEY = "android.senderKey"; + private static final String SUGGEST_SMS_BLACKLIST = "android.suggestSmsBlacklist"; /** * We will only store remote values for flags in this set. If you want a flag to be controllable @@ -110,7 +112,8 @@ public final class FeatureFlags { MEDIA_QUALITY_LEVELS, RETRY_RECEIPT_LIFESPAN, RETRY_RESPOND_MAX_AGE, - SENDER_KEY + SENDER_KEY, + SUGGEST_SMS_BLACKLIST ); @VisibleForTesting @@ -156,7 +159,8 @@ public final class FeatureFlags { MP4_GIF_SEND_SUPPORT, MEDIA_QUALITY_LEVELS, RETRY_RECEIPT_LIFESPAN, - RETRY_RESPOND_MAX_AGE + RETRY_RESPOND_MAX_AGE, + SUGGEST_SMS_BLACKLIST ); /** @@ -199,7 +203,7 @@ public final class FeatureFlags { Log.i(TAG, "init() " + REMOTE_VALUES.toString()); } - public static synchronized void refreshIfNecessary() { + public static void refreshIfNecessary() { long timeSinceLastFetch = System.currentTimeMillis() - SignalStore.remoteConfigValues().getLastFetchTime(); if (timeSinceLastFetch < 0 || timeSinceLastFetch > FETCH_INTERVAL) { @@ -210,6 +214,12 @@ public final class FeatureFlags { } } + @WorkerThread + public static void refreshSync() throws IOException { + Map config = ApplicationDependencies.getSignalServiceAccountManager().getRemoteConfig(); + FeatureFlags.update(config); + } + public static synchronized void update(@NonNull Map config) { Map memory = REMOTE_VALUES; Map disk = parseStoredConfig(SignalStore.remoteConfigValues().getPendingConfig()); @@ -333,7 +343,7 @@ public final class FeatureFlags { return getBoolean(MP4_GIF_SEND_SUPPORT, false); } - public static @Nullable String getMediaQualityLevels() { + public static @NonNull String getMediaQualityLevels() { return getString(MEDIA_QUALITY_LEVELS, ""); } @@ -352,6 +362,11 @@ public final class FeatureFlags { return getBoolean(SENDER_KEY, false); } + /** A comma-delimited list of country codes that should not be told about SMS during onboarding. */ + public static @NonNull String suggestSmsBlacklist() { + return getString(SUGGEST_SMS_BLACKLIST, ""); + } + /** Only for rendering debug info. */ public static synchronized @NonNull Map getMemoryValues() { return new TreeMap<>(REMOTE_VALUES); diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java index c415741d84..de3abd8421 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java @@ -8,11 +8,15 @@ import com.google.i18n.phonenumbers.PhoneNumberUtil; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.mms.PushMediaConstraints; +import org.thoughtcrime.securesms.phonenumbers.PhoneNumberFormatter; import org.thoughtcrime.securesms.recipients.Recipient; +import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Optional; +import java.util.Set; /** * Provide access to locale specific values within feature flags following the locale CSV-Colon format. @@ -47,6 +51,16 @@ public final class LocaleFeatureFlags { return Optional.ofNullable(PushMediaConstraints.MediaConfig.forLevel(level)); } + /** + * Whether or not you should suggest SMS during onboarding. + */ + public static boolean shouldSuggestSms() { + Set blacklist = new HashSet<>(Arrays.asList(FeatureFlags.suggestSmsBlacklist().split(","))); + String countryCode = String.valueOf(PhoneNumberFormatter.getLocalCountryCode()); + + return !blacklist.contains(countryCode); + } + /** * Parses a comma-separated list of country codes colon-separated from how many buckets out of 1 million * should be enabled to see this megaphone in that country code. At the end of the list, an optional