empathicqubit/Signal-Android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Ensure consistency and completeness of feature flag remote capable designation.

Browse source 
Make CustomVideoMuxer flag remote capable.
This commit is contained in:
Alan Evans 2021-01-07 12:01:03 -04:00
parent e850d8e917
commit dfd5b2c225
3 changed files with 98 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java
View file

@ -65,13 +65,14 @@ public final class FeatureFlags {
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";
private static final String DISABLE_CUSTOM_VIDEO_MUXER = "android.disableCustomVideoMuxer";
private static final String CUSTOM_VIDEO_MUXER = "android.customVideoMuxer";
/**
* We will only store remote values for flags in this set. If you want a flag to be controllable
* remotely, place it in here.
*/
private static final Set<String> REMOTE_CAPABLE = SetUtil.newHashSet(
@VisibleForTesting
static final Set<String> REMOTE_CAPABLE = SetUtil.newHashSet(
GROUPS_V2_RECOMMENDED_LIMIT,
GROUPS_V2_HARD_LIMIT,
INTERNAL_USER,
@ -85,7 +86,13 @@ public final class FeatureFlags {
GV1_MANUAL_MIGRATE,
GV1_FORCED_MIGRATE,
GROUP_CALLING,
SEND_VIEWED_RECEIPTS
SEND_VIEWED_RECEIPTS,
CUSTOM_VIDEO_MUXER
);
@VisibleForTesting
static final Set<String> NOT_REMOTE_CAPABLE = SetUtil.newHashSet(
PHONE_NUMBER_PRIVACY_VERSION
);
/**
@ -95,7 +102,8 @@ public final class FeatureFlags {
* an addition to this map.
*/
@SuppressWarnings("MismatchedQueryAndUpdateOfCollection")
private static final Map<String, Object> FORCED_VALUES = new HashMap<String, Object>() {{
@VisibleForTesting
static final Map<String, Object> FORCED_VALUES = new HashMap<String, Object>() {{
}};
/**
@ -105,20 +113,22 @@ public final class FeatureFlags {
* will be updated arbitrarily at runtime. This will make values more responsive, but also places
* more burden on the reader to ensure that the app experience remains consistent.
*/
private static final Set<String> HOT_SWAPPABLE = SetUtil.newHashSet(
@VisibleForTesting
static final Set<String> HOT_SWAPPABLE = SetUtil.newHashSet(
VERIFY_V2,
CLIENT_EXPIRATION,
GROUP_CALLING,
GV1_MIGRATION_JOB,
DISABLE_CUSTOM_VIDEO_MUXER
CUSTOM_VIDEO_MUXER
);
/**
* Flags in this set will stay true forever once they receive a true value from a remote config.
*/
private static final Set<String> STICKY = SetUtil.newHashSet(
@VisibleForTesting
static final Set<String> STICKY = SetUtil.newHashSet(
VERIFY_V2
);
);
/**
* Listeners that are called when the value in {@link #REMOTE_VALUES} changes. That means that
@ -257,7 +267,7 @@ public final class FeatureFlags {
/** Whether to use the custom streaming muxer or built in android muxer. */
public static boolean useStreamingVideoMuxer() {
return !getBoolean(DISABLE_CUSTOM_VIDEO_MUXER, false);
return getBoolean(CUSTOM_VIDEO_MUXER, false);
}
/** Only for rendering debug info. */

5
app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlagsTest.java
View file

@ -21,11 +21,6 @@ public class FeatureFlagsTest extends BaseUnitTest {
private static final String A = "A";
private static final String B = "B";
@Test
public void disallowForcedFlags() {
assertTrue(FeatureFlags.getForcedValues().isEmpty());
}
@Test
public void updateInternal_newValue_ignoreNotInRemoteCapable() {
UpdateResult result = FeatureFlags.updateInternal(mapOf(A, true,

79
app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlags_ConsistencyTest.java Normal file
View file

@ -0,0 +1,79 @@
package org.thoughtcrime.securesms.util;
import com.annimon.stream.Collectors;
import com.annimon.stream.Stream;
import org.junit.Test;
import java.util.Set;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public final class FeatureFlags_ConsistencyTest {
/**
* Ensures developer makes decision on whether a flag should or should not be remote capable.
*/
@Test
public void no_flags_are_in_both_lists() {
Set<String> intersection = SetUtil.intersection(FeatureFlags.REMOTE_CAPABLE,
FeatureFlags.NOT_REMOTE_CAPABLE);
assertTrue(intersection.isEmpty());
}
/**
* Ensures developer makes decision on whether a flag should or should not be remote capable.
*/
@Test
public void all_flags_are_in_one_list_or_another() {
Set<String> flagsByReflection = Stream.of(FeatureFlags.class.getDeclaredFields())
.filter(f -> f.getType() == String.class)
.filter(f -> !f.getName().equals("TAG"))
.map(f -> {
try {
f.setAccessible(true);
return (String) f.get(null);
} catch (IllegalAccessException e) {
throw new AssertionError(e);
}
})
.collect(Collectors.toSet());
Set<String> flagsInBothSets = SetUtil.union(FeatureFlags.REMOTE_CAPABLE,
FeatureFlags.NOT_REMOTE_CAPABLE);
assertEquals(flagsInBothSets, flagsByReflection);
}
/**
* Ensures we don't leave old feature flag values in the hot swap list.
*/
@Test
public void all_hot_swap_values_are_defined_capable_or_not() {
Set<String> flagsInBothSets = SetUtil.union(FeatureFlags.REMOTE_CAPABLE,
FeatureFlags.NOT_REMOTE_CAPABLE);
assertTrue(flagsInBothSets.containsAll(FeatureFlags.HOT_SWAPPABLE));
}
/**
* Ensures we don't leave old feature flag values in the sticky list.
*/
@Test
public void all_sticky_values_are_defined_capable_or_not() {
Set<String> flagsInBothSets = SetUtil.union(FeatureFlags.REMOTE_CAPABLE,
FeatureFlags.NOT_REMOTE_CAPABLE);
assertTrue(flagsInBothSets.containsAll(FeatureFlags.STICKY));
}
/**
* Ensures we don't release with forced values which is intended for local development only.
*/
@Test
public void no_values_are_forced() {
assertTrue(FeatureFlags.FORCED_VALUES.isEmpty());
}
}