Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2025-12-23 12:38:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve FeatureFlag change detection, use for SVR3.

Browse Source
This commit is contained in:
Greyson Parrelli
2024-06-12 14:01:36 -04:00
parent 13f7a64139
commit ecbea9fd95
2 changed files with 45 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.kt
View File

@@ -9,10 +9,9 @@ import org.signal.core.util.logging.Log
import org.signal.core.util.mebiBytes import org.signal.core.util.mebiBytes
import org.thoughtcrime.securesms.BuildConfig import org.thoughtcrime.securesms.BuildConfig
import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.dependencies.AppDependencies
import org.thoughtcrime.securesms.dependencies.AppDependencies.application
import org.thoughtcrime.securesms.dependencies.AppDependencies.jobManager
import org.thoughtcrime.securesms.groups.SelectionLimits import org.thoughtcrime.securesms.groups.SelectionLimits
import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob
import org.thoughtcrime.securesms.jobs.Svr3MirrorJob
import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.keyvalue.SignalStore
import org.thoughtcrime.securesms.messageprocessingalarm.RoutineMessageFetchReceiver import org.thoughtcrime.securesms.messageprocessingalarm.RoutineMessageFetchReceiver
import java.io.IOException import java.io.IOException
@@ -67,7 +66,7 @@ object FeatureFlags {
if (timeSinceLastFetch < 0 || timeSinceLastFetch > FETCH_INTERVAL.inWholeMilliseconds) { if (timeSinceLastFetch < 0 || timeSinceLastFetch > FETCH_INTERVAL.inWholeMilliseconds) {
Log.i(TAG, "Scheduling remote config refresh.") Log.i(TAG, "Scheduling remote config refresh.")
jobManager.add(RemoteConfigRefreshJob()) AppDependencies.jobManager.add(RemoteConfigRefreshJob())
} else { } else {
Log.i(TAG, "Skipping remote config refresh. Refreshed $timeSinceLastFetch ms ago.") Log.i(TAG, "Skipping remote config refresh. Refreshed $timeSinceLastFetch ms ago.")
} }
@@ -194,31 +193,19 @@ object FeatureFlags {
@JvmStatic @JvmStatic
@VisibleForTesting @VisibleForTesting
fun computeChanges(oldMap: Map<String, Any>, newMap: Map<String, Any>): Map<String, Change> { fun computeChanges(oldMap: Map<String, Any>, newMap: Map<String, Any>): Map<String, ConfigChange> {
val changes: MutableMap<String, Change> = mutableMapOf() val allKeys: Set<String> = oldMap.keys + newMap.keys
val allKeys: MutableSet<String> = mutableSetOf()
allKeys += oldMap.keys return allKeys
allKeys += newMap.keys .filter { oldMap[it] != newMap[it] }
.associateWith { key ->
for (key in allKeys) { ConfigChange(
val oldValue = oldMap[key] oldValue = oldMap[key],
val newValue = newMap[key] newValue = newMap[key]
)
if (oldValue == null && newValue == null) {
throw AssertionError("Should not be possible.")
} else if (oldValue != null && newValue == null) {
changes[key] = Change.REMOVED
} else if (newValue !== oldValue && newValue is Boolean) {
changes[key] = if (newValue) Change.ENABLED else Change.DISABLED
} else if (oldValue != newValue) {
changes[key] = Change.CHANGED
} }
} }
return changes
}
private fun parseStoredConfig(stored: String?): Map<String, Any> { private fun parseStoredConfig(stored: String?): Map<String, Any> {
val parsed: MutableMap<String, Any> = HashMap() val parsed: MutableMap<String, Any> = HashMap()
@@ -256,7 +243,7 @@ object FeatureFlags {
} }
} }
private fun triggerFlagChangeListeners(changes: Map<String, Change>) { private fun triggerFlagChangeListeners(changes: Map<String, ConfigChange>) {
for ((key, value) in changes) { for ((key, value) in changes) {
val listener = configsByKey[key]?.onChangeListener val listener = configsByKey[key]?.onChangeListener
@@ -271,16 +258,14 @@ object FeatureFlags {
class UpdateResult( class UpdateResult(
val memory: Map<String, Any>, val memory: Map<String, Any>,
val disk: Map<String, Any>, val disk: Map<String, Any>,
val memoryChanges: Map<String, Change> val memoryChanges: Map<String, ConfigChange>
) )
data class ConfigChange(val oldValue: Any?, val newValue: Any?)
@VisibleForTesting @VisibleForTesting
fun interface OnFlagChange { fun interface OnFlagChange {
fun onFlagChange(change: Change) fun onFlagChange(change: ConfigChange)
}
enum class Change {
ENABLED, DISABLED, CHANGED, REMOVED
} }
// endregion // endregion
@@ -1072,7 +1057,7 @@ object FeatureFlags {
val backgroundMessageProcessInterval: Long by remoteValue( val backgroundMessageProcessInterval: Long by remoteValue(
key = "android.messageProcessor.alarmIntervalMins", key = "android.messageProcessor.alarmIntervalMins",
hotSwappable = true, hotSwappable = true,
onChangeListener = { RoutineMessageFetchReceiver.startOrUpdateAlarm(application) } onChangeListener = { RoutineMessageFetchReceiver.startOrUpdateAlarm(AppDependencies.application) }
) { value -> ) { value ->
val inMinutes = value.asLong(6.hours.inWholeMinutes) val inMinutes = value.asLong(6.hours.inWholeMinutes)
inMinutes.minutes.inWholeMilliseconds inMinutes.minutes.inWholeMilliseconds
@@ -1098,9 +1083,14 @@ object FeatureFlags {
val svr3MigrationPhase: Int by remoteInt( val svr3MigrationPhase: Int by remoteInt(
key = "global.svr3.phase", key = "global.svr3.phase",
defaultValue = 0, defaultValue = 0,
hotSwappable = true hotSwappable = true,
onChangeListener = {
if ((it.oldValue == null || it.oldValue == 0) && it.newValue == 1) {
Log.w(TAG, "Detected the SVR3 migration phase change to 1! Enqueuing a mirroring job.")
AppDependencies.jobManager.add(Svr3MirrorJob())
}
}
) )
// TODO [svr3] on change listener to enqueue migration
// endregion // endregion
} }

42
app/src/test/java/org/thoughtcrime/securesms/util/FeatureFlagsTest.java
View File

@@ -2,7 +2,7 @@ package org.thoughtcrime.securesms.util;
import org.junit.Test; import org.junit.Test;
import org.thoughtcrime.securesms.BaseUnitTest; import org.thoughtcrime.securesms.BaseUnitTest;
import org.thoughtcrime.securesms.util.FeatureFlags.Change; import org.thoughtcrime.securesms.util.FeatureFlags.ConfigChange;
import org.thoughtcrime.securesms.util.FeatureFlags.UpdateResult; import org.thoughtcrime.securesms.util.FeatureFlags.UpdateResult;
import java.util.Arrays; import java.util.Arrays;
@@ -61,7 +61,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getMemory());
assertEquals(mapOf(A, true), result.getDisk()); assertEquals(mapOf(A, true), result.getDisk());
assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(null, true), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -75,7 +75,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(A, 1), result.getMemory()); assertEquals(mapOf(A, 1), result.getMemory());
assertEquals(mapOf(A, 1), result.getDisk()); assertEquals(mapOf(A, 1), result.getDisk());
assertEquals(Change.CHANGED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(null, 1), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -103,7 +103,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getMemory());
assertEquals(mapOf(A, true), result.getDisk()); assertEquals(mapOf(A, true), result.getDisk());
assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(null, true), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -145,7 +145,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getMemory());
assertEquals(mapOf(A, true), result.getDisk()); assertEquals(mapOf(A, true), result.getDisk());
assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(false, true), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -159,7 +159,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(A, 2), result.getMemory()); assertEquals(mapOf(A, 2), result.getMemory());
assertEquals(mapOf(A, 2), result.getDisk()); assertEquals(mapOf(A, 2), result.getDisk());
assertEquals(Change.CHANGED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(1, 2), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -173,7 +173,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(A, true), result.getMemory()); assertEquals(mapOf(A, true), result.getMemory());
assertEquals(mapOf(A, true), result.getDisk()); assertEquals(mapOf(A, true), result.getDisk());
assertEquals(Change.ENABLED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(false, true), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -229,7 +229,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getMemory());
assertEquals(mapOf(), result.getDisk()); assertEquals(mapOf(), result.getDisk());
assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(true, null), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -243,7 +243,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getMemory());
assertEquals(mapOf(), result.getDisk()); assertEquals(mapOf(), result.getDisk());
assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(true, null), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -327,7 +327,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getMemory());
assertEquals(mapOf(), result.getDisk()); assertEquals(mapOf(), result.getDisk());
assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(false, null), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -341,7 +341,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
assertEquals(mapOf(), result.getMemory()); assertEquals(mapOf(), result.getMemory());
assertEquals(mapOf(), result.getDisk()); assertEquals(mapOf(), result.getDisk());
assertEquals(Change.REMOVED, result.getMemoryChanges().get(A)); assertEquals(new ConfigChange(true, null), result.getMemoryChanges().get(A));
} }
@Test @Test
@@ -378,7 +378,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
@Test @Test
public void computeChanges_generic() { public void computeChanges_generic() {
Map<String, Object> oldMap = new HashMap<String, Object>() {{ Map<String, Object> oldMap = new HashMap<>() {{
put("a", true); put("a", true);
put("b", false); put("b", false);
put("c", true); put("c", true);
@@ -389,7 +389,7 @@ public class FeatureFlagsTest extends BaseUnitTest {
put("j", "gwen"); put("j", "gwen");
}}; }};
Map<String, Object> newMap = new HashMap<String, Object>() {{ Map<String, Object> newMap = new HashMap<>() {{
put("a", true); put("a", true);
put("b", true); put("b", true);
put("c", false); put("c", false);
@@ -400,17 +400,17 @@ public class FeatureFlagsTest extends BaseUnitTest {
put("j", "stacy"); put("j", "stacy");
}}; }};
Map<String, Change> changes = FeatureFlags.computeChanges(oldMap, newMap); Map<String, ConfigChange> changes = FeatureFlags.computeChanges(oldMap, newMap);
assertFalse(changes.containsKey("a")); assertFalse(changes.containsKey("a"));
assertEquals(Change.ENABLED, changes.get("b")); assertEquals(new ConfigChange(false, true), changes.get("b"));
assertEquals(Change.DISABLED, changes.get("c")); assertEquals(new ConfigChange(true, false), changes.get("c"));
assertEquals(Change.REMOVED, changes.get("d")); assertEquals(new ConfigChange(false, null), changes.get("d"));
assertEquals(Change.ENABLED, changes.get("e")); assertEquals(new ConfigChange(null, true), changes.get("e"));
assertEquals(Change.DISABLED, changes.get("f")); assertEquals(new ConfigChange(null, false), changes.get("f"));
assertEquals(Change.CHANGED, changes.get("h")); assertEquals(new ConfigChange(5, 7), changes.get("h"));
assertFalse(changes.containsKey("i")); assertFalse(changes.containsKey("i"));
assertEquals(Change.CHANGED, changes.get("j")); assertEquals(new ConfigChange("gwen", "stacy"), changes.get("j"));
} }
private static <V> Set<V> setOf(V... values) { private static <V> Set<V> setOf(V... values) {